From 1d07dd6b3860cb5999c8417542fdbb94df310241 Mon Sep 17 00:00:00 2001 From: Joao Carreira Date: Fri, 22 Sep 2017 00:54:46 +0100 Subject: [PATCH] Improve buffering and fix memory leaks Use a dedicated class to handle frame buffer Fix memory leak in stream handler and frame allocation --- app/ConfigureFormatDialog.cpp | 2 +- app/VideoSubWindow.cpp | 59 +++++++---- app/VideoSubWindow.h | 1 + lib/PlaYUVerFrame.cpp | 33 +++--- lib/PlaYUVerStream.cpp | 175 +++++++++++++------------------ lib/PlaYUVerStream.h | 15 ++- lib/PlaYUVerStreamHandlerIf.h | 17 +-- lib/StreamHandlerLibav.cpp | 2 + lib/StreamHandlerOpenCV.cpp | 3 + lib/StreamHandlerPortableMap.cpp | 2 + lib/StreamHandlerRaw.cpp | 4 + tools/PlaYUVerTools.cpp | 6 +- 12 files changed, 159 insertions(+), 160 deletions(-) diff --git a/app/ConfigureFormatDialog.cpp b/app/ConfigureFormatDialog.cpp index 3e3c1f0..dd8941e 100644 --- a/app/ConfigureFormatDialog.cpp +++ b/app/ConfigureFormatDialog.cpp @@ -347,7 +347,7 @@ ConfigureFormatDialog::ConfigureFormatDialog( QWidget* parent ) : QDialog( paren m_spinBoxFrameRate = new QSpinBox(); m_spinBoxFrameRate->setSuffix( QStringLiteral( " Hz" ) ); m_spinBoxFrameRate->setMinimumWidth( 30 ); - m_spinBoxFrameRate->setRange( 0, 1000 ); + m_spinBoxFrameRate->setRange( 0, 1000 ); m_spinBoxFrameRate->setValue( 30 ); framerateFormatLayout->addWidget( framerateFormatLabel ); framerateFormatLayout->addItem( diff --git a/app/VideoSubWindow.cpp b/app/VideoSubWindow.cpp index 242aaa3..e76b94b 100644 --- a/app/VideoSubWindow.cpp +++ b/app/VideoSubWindow.cpp @@ -689,6 +689,33 @@ Void VideoSubWindow::refreshFrame( Bool bThreaded ) } } +Bool VideoSubWindow::goToNextFrame( Bool bThreaded ) +{ +#ifndef QT_NO_CONCURRENT + m_cRefreshResult.waitForFinished(); + m_cReadResult.waitForFinished(); +#endif + Bool bEndOfSeq = m_pCurrStream->setNextFrame(); + if( !bEndOfSeq ) + { +#ifndef QT_NO_CONCURRENT + m_cRefreshResult.waitForFinished(); + if( bThreaded ) + { + refreshFrame(); + m_cReadResult = + QtConcurrent::run( m_pCurrStream, &PlaYUVerStream::readNextFrameFillRGBBuffer ); + } + else +#endif + { + refreshFrame(); + m_pCurrStream->readNextFrameFillRGBBuffer(); + } + } + return bEndOfSeq; +} + Bool VideoSubWindow::save( QString filename ) { Bool iRet = false; @@ -727,20 +754,7 @@ Bool VideoSubWindow::playEvent() Bool bEndOfSeq = false; if( m_pCurrStream && m_bIsPlaying ) { -#ifndef QT_NO_CONCURRENT - m_cRefreshResult.waitForFinished(); - m_cReadResult.waitForFinished(); -#endif - bEndOfSeq = m_pCurrStream->setNextFrame(); - if( !bEndOfSeq ) - { - refreshFrame(); -#ifndef QT_NO_CONCURRENT - m_cReadResult = QtConcurrent::run( m_pCurrStream, &PlaYUVerStream::readNextFrameFillRGBBuffer ); -#else - m_pCurrStream->readNextFrameFillRGBBuffer(); -#endif - } + bEndOfSeq = goToNextFrame( true ); } return bEndOfSeq; } @@ -754,22 +768,27 @@ Void VideoSubWindow::pause() Void VideoSubWindow::seekAbsoluteEvent( UInt new_frame_num ) { if( m_pCurrStream ) - { if( m_pCurrStream->seekInput( new_frame_num ) ) - { refreshFrame(); - } - } } Void VideoSubWindow::seekRelativeEvent( Bool bIsFoward ) { + Bool bRefresh = false; if( m_pCurrStream ) { - if( m_pCurrStream->seekInputRelative( bIsFoward ) ) + if( bIsFoward ) { - refreshFrame(); + goToNextFrame( true ); } + else + { + bRefresh = m_pCurrStream->seekInputRelative( bIsFoward ); + } + } + if( bRefresh ) + { + refreshFrame(); } } diff --git a/app/VideoSubWindow.h b/app/VideoSubWindow.h index 7507b31..913ae2c 100644 --- a/app/VideoSubWindow.h +++ b/app/VideoSubWindow.h @@ -101,6 +101,7 @@ private: QFuture m_cReadResult; Void refreshFrameOperation(); + Bool goToNextFrame( Bool bThreaded = false ); static Bool guessFormat( QString filename, UInt& rWidth, diff --git a/lib/PlaYUVerFrame.cpp b/lib/PlaYUVerFrame.cpp index 367f885..2dc4054 100644 --- a/lib/PlaYUVerFrame.cpp +++ b/lib/PlaYUVerFrame.cpp @@ -214,6 +214,24 @@ struct PlaYUVerFramePrivate m_bInit = true; } + + ~PlaYUVerFramePrivate() + { + if( m_puiHistogram ) + { + freeMem1D( m_puiHistogram ); + } + m_puiHistogram = NULL; + m_bHasHistogram = false; + + if( m_pppcInputPel ) + freeImgMemory( m_pppcInputPel ); + m_pppcInputPel = NULL; + + if( m_pcARGB32 ) + freeMem1D( m_pcARGB32 ); + m_pcARGB32 = NULL; + } }; /*! @@ -302,21 +320,6 @@ PlaYUVerFrame::PlaYUVerFrame( const PlaYUVerFrame* other, PlaYUVerFrame::~PlaYUVerFrame() { - if( d->m_puiHistogram ) - { - freeMem1D( d->m_puiHistogram ); - } - d->m_puiHistogram = NULL; - d->m_bHasHistogram = false; - - if( d->m_pppcInputPel ) - freeImgMemory( d->m_pppcInputPel ); - d->m_pppcInputPel = NULL; - - if( d->m_pcARGB32 ) - freeMem1D( d->m_pcARGB32 ); - d->m_pcARGB32 = NULL; - delete d; } diff --git a/lib/PlaYUVerStream.cpp b/lib/PlaYUVerStream.cpp index 0c4924c..b368929 100644 --- a/lib/PlaYUVerStream.cpp +++ b/lib/PlaYUVerStream.cpp @@ -45,63 +45,47 @@ class PlaYUVerStreamBufferPrivate { private: - UInt m_uiSize; - UInt m_uiIndex; std::vector m_apcFrameBuffer; + UInt m_uiIndex; public: PlaYUVerStreamBufferPrivate( UInt size, UInt width, UInt height, Int pelFormat, Int bitsPixel ) { - m_uiSize = size; - for( UInt i = 0; i < m_uiSize; i++ ) + for( UInt i = 0; i < size; i++ ) { - try - { - PlaYUVerFrame* pFrame = new PlaYUVerFrame( width, height, pelFormat, bitsPixel ); - m_apcFrameBuffer.push_back( pFrame ); - } - catch( PlaYUVerFailure& e ) - { - throw PlaYUVerFailure( "PlaYUVerStream", "Cannot allocated frame buffer" ); - } + PlaYUVerFrame* pFrame = new PlaYUVerFrame( width, height, pelFormat, bitsPixel ); + m_apcFrameBuffer.push_back( pFrame ); } - m_uiIndex = 0; + m_uiIndex = 0; } ~PlaYUVerStreamBufferPrivate() { - for( UInt i = 0; i < m_apcFrameBuffer.size(); i++ ) + while( m_apcFrameBuffer.size() > 0 ) { delete m_apcFrameBuffer.back(); m_apcFrameBuffer.pop_back(); } } + Void increase( UInt newSize ) + { + for( UInt i = m_apcFrameBuffer.size(); i < newSize; i++ ) + { + PlaYUVerFrame* pFrame = new PlaYUVerFrame( m_apcFrameBuffer.at( 0 ) ); + m_apcFrameBuffer.push_back( pFrame ); + } + } + UInt size() { return m_apcFrameBuffer.size(); } + Void setIndex( UInt i ) { m_uiIndex = i; } + PlaYUVerFrame* frame( Int i ) { return m_apcFrameBuffer.at( i ); } - UInt size() - { - return m_uiSize; - } - Void setIndex( UInt i ) - { - m_uiSize = i; - } - PlaYUVerFrame* frame( Int i) { return m_apcFrameBuffer.at( i ); } + PlaYUVerFrame* current() { return m_apcFrameBuffer.at( m_uiIndex ); } + PlaYUVerFrame* next() { return m_apcFrameBuffer.at( nextIndex() ); } + Void setNextFrame() { m_uiIndex = nextIndex(); } - PlaYUVerFrame* current() { return m_apcFrameBuffer.at( m_uiIndex ); } - PlaYUVerFrame* next() { return m_apcFrameBuffer.at( nextIndex() ); } - Void setNextFrame() - { - m_uiIndex = nextIndex(); - } private: - inline Int nextIndex() - { - return m_uiIndex + 1 >= m_uiSize ? 0 : m_uiIndex + 1; - } - inline Int prevIndex() - { - return m_uiIndex - 1 < 0 ? 0 : m_uiIndex + 1; - } + inline Int nextIndex() { return m_uiIndex + 1 >= m_apcFrameBuffer.size() ? 0 : m_uiIndex + 1; } + inline Int prevIndex() { return m_uiIndex - 1 < 0 ? m_apcFrameBuffer.size() - 1 : m_uiIndex - 1; } }; std::vector PlaYUVerStream::supportedReadFormats() @@ -200,7 +184,6 @@ PlaYUVerStream::PlaYUVerStream() m_uiTotalFrameNum = 0; m_iCurrFrameNum = -1; m_cFilename = ""; - m_uiCurrFrameFileIdx = 0; } PlaYUVerStream::~PlaYUVerStream() @@ -450,35 +433,23 @@ Void PlaYUVerStream::loadAll() if( m_bLoadAll || !m_bIsInput ) return; - - // Keep past, current and future frames - - PlaYUVerStreamBufferPrivate* aux_frameBuffer; - try - { - aux_frameBuffer = new PlaYUVerStreamBufferPrivate( - m_uiTotalFrameNum, m_pcHandler->m_uiWidth, m_pcHandler->m_uiHeight, - m_pcHandler->m_iPixelFormat, m_pcHandler->m_uiBitsPerPixel ); - } - catch( PlaYUVerFailure& e ) - { - delete aux_frameBuffer; - return; - } - - if( m_frameBuffer ) - { - delete m_frameBuffer; - } - m_frameBuffer = aux_frameBuffer; + try + { + m_frameBuffer->increase( m_uiTotalFrameNum ); + } + catch( PlaYUVerFailure& e ) + { + close(); + throw PlaYUVerFailure( "PlaYUVerStream", "Cannot allocated frame buffer" ); + } seekInput( 0 ); - for( UInt i = 2; i < m_frameBuffer->size(); i++ ) + for( UInt i = 2; i < m_frameBuffer->size(); i++ ) { - readFrame( m_frameBuffer->frame(i) ); - } + readFrame( m_frameBuffer->frame( i ) ); + } m_bLoadAll = true; - m_iCurrFrameNum = 0; + m_iCurrFrameNum = 0; } Void PlaYUVerStream::getDuration( Int* duration_array ) @@ -504,26 +475,25 @@ Void PlaYUVerStream::getDuration( Int* duration_array ) // *duration_array++ = secs; } -Bool PlaYUVerStream::readFrame(PlaYUVerFrame *frame) +Bool PlaYUVerStream::readFrame( PlaYUVerFrame* frame ) { - if( !m_bInit || !m_bIsInput || m_uiCurrFrameFileIdx >= m_uiTotalFrameNum ) - return false; + if( !m_bInit || !m_bIsInput || m_pcHandler->m_uiCurrFrameFileIdx >= m_uiTotalFrameNum ) + return false; - if( m_bLoadAll ) - return true; + if( m_bLoadAll ) + return true; - if( !m_pcHandler->read( frame ) ) + if( !m_pcHandler->read( frame ) ) { throw PlaYUVerFailure( "PlaYUVerStream", "Cannot read file" ); - return false; + return false; } - m_uiCurrFrameFileIdx++; - return true; + return true; } Void PlaYUVerStream::writeFrame() { - writeFrame( m_frameBuffer->current() ); + writeFrame( m_frameBuffer->current() ); } Void PlaYUVerStream::writeFrame( PlaYUVerFrame* pcFrame ) @@ -537,7 +507,7 @@ Void PlaYUVerStream::writeFrame( PlaYUVerFrame* pcFrame ) Bool PlaYUVerStream::saveFrame( const String& filename ) { - return saveFrame( filename, m_frameBuffer->current() ); + return saveFrame( filename, m_frameBuffer->current() ); } Bool PlaYUVerStream::saveFrame( const String& filename, PlaYUVerFrame* saveFrame ) @@ -558,44 +528,42 @@ Bool PlaYUVerStream::setNextFrame() { Bool bEndOfSeq = false; - if( m_iCurrFrameNum + 1 < Int( m_uiTotalFrameNum ) ) - { - m_frameBuffer->setNextFrame(); - m_iCurrFrameNum++; - } - else - { - bEndOfSeq = true; - } + if( m_iCurrFrameNum + 1 < Int( m_uiTotalFrameNum ) ) + { + m_frameBuffer->setNextFrame(); + m_iCurrFrameNum++; + } + else + { + bEndOfSeq = true; + } return bEndOfSeq; } Void PlaYUVerStream::readNextFrame() { - readFrame( m_frameBuffer->next() ); + readFrame( m_frameBuffer->next() ); } Void PlaYUVerStream::readNextFrameFillRGBBuffer() { - readNextFrame(); - m_frameBuffer->next()->fillRGBBuffer(); - return; + readNextFrame(); + m_frameBuffer->next()->fillRGBBuffer(); + return; } - - PlaYUVerFrame* PlaYUVerStream::getCurrFrame( PlaYUVerFrame* pyuv_image ) { if( pyuv_image == NULL ) - pyuv_image = new PlaYUVerFrame( m_frameBuffer->current() ); - else - pyuv_image->copyFrom( m_frameBuffer->current() ); + pyuv_image = new PlaYUVerFrame( m_frameBuffer->current() ); + else + pyuv_image->copyFrom( m_frameBuffer->current() ); return pyuv_image; } PlaYUVerFrame* PlaYUVerStream::getCurrFrame() { - return m_frameBuffer->current(); + return m_frameBuffer->current(); } Bool PlaYUVerStream::seekInputRelative( Bool bIsFoward ) @@ -607,7 +575,7 @@ Bool PlaYUVerStream::seekInputRelative( Bool bIsFoward ) if( bIsFoward ) { bRet = !setNextFrame(); - readFrame( m_frameBuffer->next() ); + readFrame( m_frameBuffer->next() ); } else { @@ -622,26 +590,23 @@ Bool PlaYUVerStream::seekInput( UInt64 new_frame_num ) if( !m_bInit || new_frame_num >= m_uiTotalFrameNum || (Int64)new_frame_num == m_iCurrFrameNum ) return false; - m_iCurrFrameNum = new_frame_num - 1; + m_iCurrFrameNum = new_frame_num; if( m_bLoadAll ) { - m_frameBuffer->setIndex( new_frame_num ); - setNextFrame(); + m_frameBuffer->setIndex( m_iCurrFrameNum ); return true; } - if( !m_pcHandler->seek( new_frame_num ) ) + if( !m_pcHandler->seek( m_iCurrFrameNum ) ) { throw PlaYUVerFailure( "PlaYUVerStream", "Cannot write into the file" ); } - m_uiCurrFrameFileIdx = new_frame_num; - - m_frameBuffer->setIndex( 0 ); - readFrame( m_frameBuffer->current() ); - if( m_uiTotalFrameNum > 1 ) - readFrame( m_frameBuffer->next() ); + m_frameBuffer->setIndex( 0 ); + readFrame( m_frameBuffer->current() ); + if( m_uiTotalFrameNum > 1 ) + readFrame( m_frameBuffer->next() ); return true; } diff --git a/lib/PlaYUVerStream.h b/lib/PlaYUVerStream.h index 1570ff9..f412482 100644 --- a/lib/PlaYUVerStream.h +++ b/lib/PlaYUVerStream.h @@ -134,7 +134,6 @@ public: Void loadAll(); - Void writeFrame(); Void writeFrame( PlaYUVerFrame* pcFrame ); @@ -143,11 +142,11 @@ public: PlaYUVerFrame* getCurrFrame( PlaYUVerFrame* ); - // continuous read control - Bool setNextFrame(); - Void readNextFrame(); - Void readNextFrameFillRGBBuffer(); - PlaYUVerFrame* getCurrFrame(); + // continuous read control + Bool setNextFrame(); + Void readNextFrame(); + Void readNextFrameFillRGBBuffer(); + PlaYUVerFrame* getCurrFrame(); Bool seekInputRelative( Bool bIsFoward ); Bool seekInput( UInt64 new_frame_num ); @@ -156,7 +155,7 @@ public: Void getDuration( Int* duration_array ); private: - Bool readFrame( PlaYUVerFrame* frame ); + Bool readFrame( PlaYUVerFrame* frame ); private: Bool m_bInit; @@ -171,8 +170,6 @@ private: Bool m_bLoadAll; PlaYUVerStreamBufferPrivate* m_frameBuffer; - - UInt64 m_uiCurrFrameFileIdx; }; #endif // __PLAYUVERSTREAM_H__ diff --git a/lib/PlaYUVerStreamHandlerIf.h b/lib/PlaYUVerStreamHandlerIf.h index 0a6dfa3..26d8d6e 100644 --- a/lib/PlaYUVerStreamHandlerIf.h +++ b/lib/PlaYUVerStreamHandlerIf.h @@ -45,14 +45,16 @@ class PlaYUVerStreamHandlerIf public: PlaYUVerStreamHandlerIf() + : m_bIsInput( true ) + , m_uiCurrFrameFileIdx( 0 ) + , m_uiWidth( 0 ) + , m_uiHeight( 0 ) + , m_iPixelFormat( -1 ) + , m_uiBitsPerPixel( 8 ) + , m_iEndianness( -1 ) + , m_dFrameRate( 30 ) + , m_pStreamBuffer( NULL ) { - m_bIsInput = true; - m_uiWidth = 0; - m_uiHeight = 0; - m_iPixelFormat = -1; - m_uiBitsPerPixel = 8; - m_iEndianness = -1; - m_dFrameRate = 30; } virtual ~PlaYUVerStreamHandlerIf() {} virtual void Delete() = 0; @@ -71,6 +73,7 @@ public: protected: Bool m_bIsInput; + UInt64 m_uiCurrFrameFileIdx; String m_cFilename; UInt m_uiWidth; UInt m_uiHeight; diff --git a/lib/StreamHandlerLibav.cpp b/lib/StreamHandlerLibav.cpp index 80804a7..c6892b0 100644 --- a/lib/StreamHandlerLibav.cpp +++ b/lib/StreamHandlerLibav.cpp @@ -389,6 +389,7 @@ Bool StreamHandlerLibav::read( PlaYUVerFrame* pcFrame ) #endif pcFrame->frameFromBuffer( m_pStreamBuffer, m_iEndianness ); + m_uiCurrFrameFileIdx++; return true; } return false; @@ -402,5 +403,6 @@ Bool StreamHandlerLibav::write( PlaYUVerFrame* pcFrame ) Bool StreamHandlerLibav::seek( UInt64 iFrameNum ) { av_seek_frame( m_cFmtCtx, m_iStreamIdx, iFrameNum, AVSEEK_FLAG_FRAME ); + m_uiCurrFrameFileIdx = iFrameNum; return true; } diff --git a/lib/StreamHandlerOpenCV.cpp b/lib/StreamHandlerOpenCV.cpp index bc64333..a40d925 100644 --- a/lib/StreamHandlerOpenCV.cpp +++ b/lib/StreamHandlerOpenCV.cpp @@ -129,6 +129,7 @@ UInt64 StreamHandlerOpenCV::calculateFrameNumber() Bool StreamHandlerOpenCV::seek( UInt64 iFrameNum ) { + // m_uiCurrFrameFileIdx = return true; } @@ -147,6 +148,8 @@ Bool StreamHandlerOpenCV::read( PlaYUVerFrame* pcFrame ) Mat cvMat = cv::imread( m_cFilename ); bRet = pcFrame->fromMat( cvMat ); } + if( bRet ) + m_uiCurrFrameFileIdx++; return bRet; } diff --git a/lib/StreamHandlerPortableMap.cpp b/lib/StreamHandlerPortableMap.cpp index 03dacc8..af27be3 100644 --- a/lib/StreamHandlerPortableMap.cpp +++ b/lib/StreamHandlerPortableMap.cpp @@ -113,6 +113,7 @@ UInt64 StreamHandlerPortableMap::calculateFrameNumber() Bool StreamHandlerPortableMap::seek( UInt64 iFrameNum ) { + // m_uiCurrFrameFileIdx = return true; } @@ -122,6 +123,7 @@ Bool StreamHandlerPortableMap::read( PlaYUVerFrame* pcFrame ) if( processed_bytes != m_uiNBytesPerFrame ) return false; pcFrame->frameFromBuffer( m_pStreamBuffer, PLAYUVER_BIG_ENDIAN ); + m_uiCurrFrameFileIdx++; return true; } diff --git a/lib/StreamHandlerRaw.cpp b/lib/StreamHandlerRaw.cpp index bf34390..08b7d1c 100644 --- a/lib/StreamHandlerRaw.cpp +++ b/lib/StreamHandlerRaw.cpp @@ -48,6 +48,8 @@ Void StreamHandlerRaw::closeHandler() { if( m_pFile ) fclose( m_pFile ); + if( m_pStreamBuffer ) + freeMem1D( m_pStreamBuffer ); } Bool StreamHandlerRaw::configureBuffer( PlaYUVerFrame* pcFrame ) @@ -70,6 +72,7 @@ Bool StreamHandlerRaw::seek( UInt64 iFrameNum ) if( m_bIsInput && m_pFile ) { fseek( m_pFile, iFrameNum >= 0 ? iFrameNum * m_uiNBytesPerFrame : 0, SEEK_SET ); + m_uiCurrFrameFileIdx = iFrameNum; return true; } return false; @@ -80,6 +83,7 @@ Bool StreamHandlerRaw::read( PlaYUVerFrame* pcFrame ) UInt64 processed_bytes = fread( m_pStreamBuffer, sizeof( Byte ), m_uiNBytesPerFrame, m_pFile ); if( processed_bytes != m_uiNBytesPerFrame ) return false; + m_uiCurrFrameFileIdx++; pcFrame->frameFromBuffer( m_pStreamBuffer, m_iEndianness ); return true; } diff --git a/tools/PlaYUVerTools.cpp b/tools/PlaYUVerTools.cpp index 269de9c..896ab4f 100644 --- a/tools/PlaYUVerTools.cpp +++ b/tools/PlaYUVerTools.cpp @@ -419,7 +419,7 @@ Int PlaYUVerTools::RateReductionOperation() abEOF = m_apcInputStreams[0]->setNextFrame(); if( !abEOF ) { - m_apcInputStreams[0]->readNextFrame(); + m_apcInputStreams[0]->readNextFrame(); } } return 0; @@ -500,7 +500,7 @@ Int PlaYUVerTools::QualityOperation() abEOF[s] = m_apcInputStreams[s]->setNextFrame(); if( !abEOF[s] ) { - m_apcInputStreams[s]->readNextFrame(); + m_apcInputStreams[s]->readNextFrame(); } } } @@ -584,7 +584,7 @@ Int PlaYUVerTools::ModuleOperation() abEOF[s] = m_apcInputStreams[s]->setNextFrame(); if( !abEOF[s] ) { - m_apcInputStreams[s]->readNextFrame(); + m_apcInputStreams[s]->readNextFrame(); } } }