Требуется помощь при утечке памяти - наличие многопоточной очереди, буфера символов и структуры - PullRequest
4 голосов
/ 13 ноября 2010

Итак, у меня есть расширенный класс очереди, который помогает многопоточность, описанную здесь .

В моих объявлениях класса у меня есть

//...
    struct VideoSample
    {   
        const unsigned char * buffer;
        int len;
    };

    ConcurrentQueue<VideoSample * > VideoSamples;

//...

struct AudioSample
{   
    const unsigned char * buffer;
    int len;
};

ConcurrentQueue<AudioSample * > AudioSamples;

//...

В моем классе у меня естьдля моего приложения требуется функция:

void VideoEncoder::AddFrameToQueue(const unsigned char *buf, int size )
{
    VideoSample * newVideoSample = new VideoSample;
    VideoSamples.try_pop(newVideoSample);

    newVideoSample->buffer = buf;
    newVideoSample->len = size;
    VideoSamples.push(newVideoSample);
    //free(newVideoSample->buffer);
    //delete newVideoSample;
}

сохранение только одного кадра в очереди.

ответ предоставлен здесь о том, как удалить структуру, в этом не помогаетслучай, потому что приложение ломает.

У меня есть подобный код для аудио-очереди.

void VideoEncoder::AddSampleToQueue(const unsigned char *buf, int size )
{
    AudioSample * newAudioSample = new AudioSample;
    newAudioSample->buffer = buf;
    newAudioSample->len = size;
    AudioSamples.push(newAudioSample);
    url_write (url_context, (unsigned char *)newAudioSample->buffer, newAudioSample->len);
    AudioSamples.wait_and_pop(newAudioSample);
    //delete newAudioSample;
}

и функция, которая выполняется в отдельном потоке:

void VideoEncoder::UrlWriteData()
{
    while(1){
        switch (AudioSamples.empty()){
        case true : 
            switch(VideoSamples.empty()){
        case true : Sleep(5); break;
        case false :    
            VideoSample * newVideoSample = new VideoSample;
            VideoSamples.wait_and_pop(newVideoSample);
            url_write (url_context, (unsigned char *)newVideoSample->buffer, newVideoSample->len);
            break;
            } break;
        case false :  Sleep(3);     break;
        }
    }
}

BTW: для потоковой передачиМедиа-данные для URL Я использую функцию ffmpeg.

Кстати: здесь код, который я использую для очередей:

#include <string>
#include <queue>
#include <iostream>

// Boost
#include <boost/thread.hpp>
#include <boost/timer.hpp>

template<typename Data>
class ConcurrentQueue
{
private:
    std::queue<Data> the_queue;
    mutable boost::mutex the_mutex;
    boost::condition_variable the_condition_variable;
public:
    void push(Data const& data)
    {
        boost::mutex::scoped_lock lock(the_mutex);
        the_queue.push(data);
        lock.unlock();
        the_condition_variable.notify_one();
    }

    bool empty() const
    {
        boost::mutex::scoped_lock lock(the_mutex);
        return the_queue.empty();
    }

    bool try_pop(Data& popped_value)
    {
        boost::mutex::scoped_lock lock(the_mutex);
        if(the_queue.empty())
        {
            return false;
        }

        popped_value=the_queue.front();
        the_queue.pop();
        return true;
    }

    void wait_and_pop(Data& popped_value)
    {
        boost::mutex::scoped_lock lock(the_mutex);
        while(the_queue.empty())
        {
            the_condition_variable.wait(lock);
        }

        popped_value=the_queue.front();
        the_queue.pop();
    }

    Data& front()
    {
        boost::mutex::scoped_lock lock(the_mutex);
        return the_queue.front();
    }

};

Мой вопрос: как очистить AddSampleToQueue и AddFrameToQueue, чтобы они не сделалиутечки памяти?

Кстати: я новичок во всех этих общих и автоматических вещах на C ++.Так сказать начинающий.Поэтому, пожалуйста, предоставьте примеры кода, которые хотя бы работают, которые интегрированы в мой код - потому что я предоставил весь свой код.Так что, если вы знаете, что делать - пожалуйста, постарайтесь интегрировать свои знания в мой пример.Спасибо.И если вы можете показать мне, как это сделать без использования общих / автоматических ptrs, я был бы очень рад.

Ответы [ 9 ]

3 голосов
/ 13 ноября 2010

Если при добавлении кадра в очередь владение массивом данных передается образцу, освободите или удалите [] его в деструкторе образца.

Возможно, вы также захотите использовать конструктор перемещения, чтобы иметь очередь ConcurrentQueue<VideoSample> вместо ConcurrentQueue<VideoSample*>, что сделало бы сэмплы, которые вы ставите в очередь и снимающие с него, автоматически.

Или, если вы управляете тем, что помещает данные в очередь, используйте вектор или boost :: array вместо массива в стиле C.

Также немного странно использовать очередь, если вы действительно хотите, чтобы в ней была только одна вещь. Вместо этого можно использовать переменную, защищенную мьютексом и условной переменной.

3 голосов
/ 24 ноября 2010

Многие другие люди предложили общие указатели.
Я не вижу причин не использовать общий указатель вместо очереди здесь. Вы разрешаете только один кадр, в конце концов. Они поддерживают фиксацию элегантно, и с небольшим боджингом могут быть сделаны безопасными, а также простыми. Я действительно не вижу возможности для циклических ссылок, которые я вижу, так что с вами должно быть все в порядке.

В качестве альтернативы, это звучит как работа для хорошего кольцевого буфера. Таким образом, вы можете просто избежать массива голых символов. Boost реализует один изящно, и с небольшим количеством примитивной синхронизации вы сможете сделать его подходящим для ваших целей. Особо следует отметить, что это сделало бы относительно простым и простое расширение приложения для обработки онлайн-данных.

Если вам интересно, я взломаю пример кода.

3 голосов
/ 13 ноября 2010

Использование Shared_ptr

3 голосов
/ 13 ноября 2010

используйте умные указатели: http://www.drdobbs.com/184401507

1 голос
/ 23 ноября 2010

Я не уверен, что я все это понимаю, но я все равно попробую.

Функция AddFrameToQueue

Так что, очевидно, вам нужен один кадр в очереди ввремя, что означает, что вам, вероятно, вообще не нужна очередь.В любом случае: либо еще нет кадра в очереди, и вы должны создать новый, либо он есть, и вы должны перезаписать его поля buffer и len:

void VideoEncoder::AddFrameToQueue(const unsigned char *buf, int size )
{
    VideoSample * newVideoSample = 0;
    if (!VideoSamples.try_pop(newVideoSample))
    {
        // Nothing in queue yet : we allocate a whole new VideoSample
        newVideoSample = new VideoSample;
    }
    else
    {
        // Here, you want to release newVideoSample->buffer depending on
        // the way it was allocated in the first place : free if malloc'ed,
        // delete if new'ed...
    }

    newVideoSample->buffer = buf;
    newVideoSample->len = size;
    VideoSamples.push(newVideoSample);

    // The VideoSample pointer has been pushed in the queue : we must no delete
    // it in order for the queue to keep containing a valid pointer
}

AddSampleToQueuefunction

Почему в конце этой функции стоит wait_and_pop: разве не должно происходить всплытие в UrlWriteData?Я действительно не понимаю эту часть.Если цель состоит в том, чтобы в очереди был один элемент, вам, вероятно, не нужна очередь (эпизод 2), но я думаю, вы могли бы использовать тот же код, что и AddFrameToQueue.

Функция UrlWriteData

Здесь данные фактически удаляются из очереди, поэтому вы хотите освободить их, как только закончите запись.

void VideoEncoder::UrlWriteData()
{
    while(1){
        switch (AudioSamples.empty()){
        case true : 
            switch(VideoSamples.empty()){
            case true : Sleep(5); break;
            case false :    
                VideoSample * newVideoSample;
                VideoSamples.wait_and_pop(newVideoSample);
                url_write (url_context, (unsigned char *)newVideoSample->buffer, newVideoSample->len);

                // Release newVideoSample->buffer using free if malloc'ed, delete
                // if new'ed...

                delete newVideoSample;

                break;
            } break;
        case false :  Sleep(3);     break;
        }
    }
}

Это лучшее, что я не могу вам сказатьне разрушая все это и не переходя на умные указатели, RAII и все те вещи, которые делают C ++:)

1 голос
/ 22 ноября 2010

измените свою функцию на приведенную ниже и сделайте аналогичные изменения в другом месте, где вы выделяете память.

void VideoEncoder :: AddFrameToQueue (const unsigned char * buf, int size) {

VideoSample * newVideoSample;
if(!VideoSamples.try_pop(newVideoSample))
{
    newVideoSample  = new VideoSample; 
}
else
{
   delete buff;
}

newVideoSample->buffer = buf; 
newVideoSample->len = size; 
VideoSamples.push(newVideoSample); 

}

И также я не могу перестать задавать этот вопрос .. Когдавам нужен только один элемент в очереди, тогда зачем вообще нужна очередь.

1 голос
/ 19 ноября 2010

Ваш код

VideoSample * newVideoSample = new VideoSample;
VideoSamples.try_pop(newVideoSample);

- это утечка памяти. Если try_pop завершится успешно, он перезапишет указатель в newVideoSample, и ваша ссылка на созданный ранее экземпляр будет потеряна навсегда!

1 голос
/ 14 ноября 2010

valgrind поможет вам найти практически любую утечку памяти в вашей программе.Хотя, как отмечали другие, вы должны использовать shared_ptrs.

1 голос
/ 13 ноября 2010

Сначала я бы изменил ConcurrentQueue<VideoSample * > VideoSamples; на

ConcurrentQueue<VideoSample> VideoSamples;

Вам не нужен этот указатель.Превратите остальные указатели в умные указатели, и все готово!

...