Нужна помощь по утечке памяти - наличие многопоточной очереди, буфера char и структуры

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

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

//...
    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;
        }
    }
}

Кстати: для потоковой передачи мультимедийных данных на 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, чтобы они не делали утечек памяти?

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


person Rella    schedule 13.11.2010    source источник
comment
swhich(somebool) {case true:... case false:...} УРА! :))   -  person Armen Tsirunyan    schedule 13.11.2010
comment
@Армен Цирунян говорят, что быстро=)   -  person Rella    schedule 13.11.2010
comment
@Armen Может быть, его реализации не хватает if/else ?   -  person Anycorn    schedule 13.11.2010
comment
@Kabimbus: Кто они такие? Они, кто убил Кенни? Кто бы они ни были, они ошибаются. А они сволочи :)   -  person Armen Tsirunyan    schedule 13.11.2010
comment
для моего приложения требуется сохранение только одного кадра в очереди. почему вы используете очередь, если вы когда-либо хотите поместить в нее только одну вещь?   -  person Pete Kirkham    schedule 13.11.2010
comment
@Pete Kirkham - это умная очередь =) обычно она содержит блокировки (очень необходимые для многопоточности)   -  person Rella    schedule 13.11.2010
comment
@Kabumbus, но если вы храните в нем только одну вещь, вы можете сделать это с помощью умной «Вещи», а не умной «Очереди».   -  person Pete Kirkham    schedule 13.11.2010


Ответы (9)


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

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); 

}

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

person bala sreekanth    schedule 22.11.2010

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

person Anycorn    schedule 13.11.2010

Используйте Shared_ptr.

person ROAR    schedule 13.11.2010
comment
Вы пытаетесь заменить все мои текущие указатели на shared_ptr? - person Rella; 13.11.2010

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

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

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

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

person Pete Kirkham    schedule 13.11.2010
comment
Вы имеете в виду добавить бесплатно и (или) удалить в ConcurrentQueue? - person Rella; 13.11.2010
comment
нет, параллельная очередь поддерживается двухсторонней очередью, поэтому она правильно уничтожает свои элементы. Но уничтожение указателя VideoSample* не удаляет указатель и не освобождает буфер. Поэтому сделайте очередь очередью из VideoSample значений и задайте деструктору удаление[] или освобождение[] данных, в зависимости от того, что было использовано для их выделения в первую очередь. - person Pete Kirkham; 13.11.2010

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

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

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

person Jake Kurzer    schedule 24.11.2010

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

ConcurrentQueue<VideoSample> VideoSamples;

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

person Philipp    schedule 13.11.2010
comment
И как поместить const unsigned char *buf в смарт/общий указатель? и нужно ли удалять его (*buf) после такой операции? - person Rella; 13.11.2010
comment
см. onlamp.com/pub/a /onlamp/2006/05/04/smart-pointers.html?page=4 для хорошего обзора. Возможно, вы сможете использовать scoped_array с const unsigned char*. - person Philipp; 13.11.2010

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

person Sam Miller    schedule 14.11.2010

Ваш код

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

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

person mmmmmmmm    schedule 19.11.2010
comment
Я вижу проблему, а не решение =) - person Rella; 19.11.2010
comment
Не инициализируйте его новым объектом. Честно говоря, если вы не можете понять проблему, вам нужно прочитать книгу по C++ с самого начала, а не просто пытаться кодировать. - person Paul; 23.11.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
}

Функция AddSampleToQueue

Почему в конце этой функции стоит 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++ :)

person icecrime    schedule 22.11.2010