Skip to content

Commit

Permalink
[omxplayer] Handle failures from EmptyThisBuffer and FillThisBuffer
Browse files Browse the repository at this point in the history 
Avoid leak of buffer as seen in PR6260
  • Loading branch information
@popcornmix
popcornmix committed Apr 15, 2015
1 parent 5df1ca1 commit 97e656c
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 62 deletions.
6 changes: 5 additions & 1 deletion xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEResamplePi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ int CActiveAEResamplePi::Resample(uint8_t **dst_buffer, int dst_samples, uint8_t
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s OMX_EmptyThisBuffer() failed with result(0x%x)", CLASSNAME, __func__, omx_err);
m_omx_mixer.DecoderEmptyBufferDone(m_omx_mixer.GetComponent(), omx_buffer);
return false;
}

Expand All @@ -518,8 +519,11 @@ int CActiveAEResamplePi::Resample(uint8_t **dst_buffer, int dst_samples, uint8_t
}
omx_err = m_omx_mixer.FillThisBuffer(m_encoded_buffer);
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s m_omx_mixer.FillThisBuffer result(0x%x)", CLASSNAME, __func__, omx_err);
m_omx_mixer.DecoderFillBufferDone(m_omx_mixer.GetComponent(), m_encoded_buffer);
return false;

}
omx_err = m_omx_mixer.WaitForOutputDone(1000);
if (omx_err != OMX_ErrorNone)
{
Expand Down
3 changes: 3 additions & 0 deletions xbmc/cores/AudioEngine/Sinks/AESinkPi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,10 @@ unsigned int CAESinkPi::AddPackets(uint8_t **data, unsigned int frames, unsigned
}
omx_err = m_omx_output->EmptyThisBuffer(omx_buffer);
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s:%s frames=%d err=%x", CLASSNAME, __func__, frames, omx_err);
m_omx_output->DecoderEmptyBufferDone(m_omx_output->GetComponent(), omx_buffer);
}
m_submitted++;
GetDelay(status);
delay = status.GetDelay();
Expand Down
42 changes: 12 additions & 30 deletions xbmc/cores/omxplayer/OMXAudio.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,12 +857,8 @@ bool COMXAudio::Initialize(AEAudioFormat format, OMXClock *clock, CDVDStreamInfo
}

omx_buffer->nOffset = 0;
omx_buffer->nFilledLen = sizeof(m_wave_header);
if(omx_buffer->nFilledLen > omx_buffer->nAllocLen)
{
CLog::Log(LOGERROR, "COMXAudio::Initialize - omx_buffer->nFilledLen > omx_buffer->nAllocLen");
return false;
}
omx_buffer->nFilledLen = std::min(sizeof(m_wave_header), omx_buffer->nAllocLen);

memset((unsigned char *)omx_buffer->pBuffer, 0x0, omx_buffer->nAllocLen);
memcpy((unsigned char *)omx_buffer->pBuffer, &m_wave_header, omx_buffer->nFilledLen);
omx_buffer->nFlags = OMX_BUFFERFLAG_CODECCONFIG | OMX_BUFFERFLAG_ENDOFFRAME;
Expand All @@ -871,6 +867,7 @@ bool COMXAudio::Initialize(AEAudioFormat format, OMXClock *clock, CDVDStreamInfo
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return false;
}
}
Expand All @@ -888,12 +885,7 @@ bool COMXAudio::Initialize(AEAudioFormat format, OMXClock *clock, CDVDStreamInfo
}

omx_buffer->nOffset = 0;
omx_buffer->nFilledLen = m_extrasize;
if(omx_buffer->nFilledLen > omx_buffer->nAllocLen)
{
CLog::Log(LOGERROR, "%s::%s - omx_buffer->nFilledLen > omx_buffer->nAllocLen", CLASSNAME, __func__);
return false;
}
omx_buffer->nFilledLen = std::min((OMX_U32)m_extrasize, omx_buffer->nAllocLen);

memset((unsigned char *)omx_buffer->pBuffer, 0x0, omx_buffer->nAllocLen);
memcpy((unsigned char *)omx_buffer->pBuffer, m_extradata, omx_buffer->nFilledLen);
Expand All @@ -903,6 +895,7 @@ bool COMXAudio::Initialize(AEAudioFormat format, OMXClock *clock, CDVDStreamInfo
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return false;
}
}
Expand Down Expand Up @@ -1222,26 +1215,14 @@ unsigned int COMXAudio::AddPackets(const void* data, unsigned int len, double dt
if(demuxer_samples_sent == demuxer_samples)
omx_buffer->nFlags |= OMX_BUFFERFLAG_ENDOFFRAME;

int nRetry = 0;
while(true)
omx_err = m_omx_decoder.EmptyThisBuffer(omx_buffer);
if (omx_err != OMX_ErrorNone)
{
omx_err = m_omx_decoder.EmptyThisBuffer(omx_buffer);
if (omx_err == OMX_ErrorNone)
{
//CLog::Log(LOGINFO, "AudiD: dts:%.0f pts:%.0f size:%d\n", dts, pts, len);
break;
}
else
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
nRetry++;
}
if(nRetry == 5)
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() finaly failed\n", CLASSNAME, __func__);
return 0;
}
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() finaly failed\n", CLASSNAME, __func__);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return 0;
}
//CLog::Log(LOGINFO, "AudiD: dts:%.0f pts:%.0f size:%d\n", dts, pts, len);

omx_err = m_omx_decoder.WaitForEvent(OMX_EventPortSettingsChanged, 0);
if (omx_err == OMX_ErrorNone)
Expand Down Expand Up @@ -1476,6 +1457,7 @@ void COMXAudio::SubmitEOS()
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return;
}
CLog::Log(LOGINFO, "%s::%s", CLASSNAME, __func__);
Expand Down
28 changes: 24 additions & 4 deletions xbmc/cores/omxplayer/OMXImage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,7 @@ bool COMXImageDec::HandlePortSettingChange(unsigned int resize_width, unsigned i
if(omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s m_omx_resize FillThisBuffer result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_resize.DecoderFillBufferDone(m_omx_resize.GetComponent(), m_decoded_buffer);
return false;
}
}
Expand Down Expand Up @@ -1174,6 +1175,7 @@ bool COMXImageDec::Decode(const uint8_t *demuxer_content, unsigned demuxer_bytes
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return false;
}
}
Expand Down Expand Up @@ -1240,8 +1242,17 @@ COMXImageEnc::~COMXImageEnc()

OMX_INIT_STRUCTURE(m_encoded_format);
m_encoded_buffer = NULL;
if(m_omx_encoder.IsInitialized())
m_omx_encoder.Deinitialize();
if (!m_success)
{
if(m_omx_encoder.IsInitialized())
{
m_omx_encoder.SetStateForComponent(OMX_StateIdle);
m_omx_encoder.FlushAll();
m_omx_encoder.FreeInputBuffers();
m_omx_encoder.FreeOutputBuffers();
m_omx_encoder.Deinitialize();
}
}
limit_calls_leave();
}

Expand Down Expand Up @@ -1383,6 +1394,7 @@ bool COMXImageEnc::Encode(unsigned char *buffer, int size, unsigned width, unsig
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_encoder.DecoderEmptyBufferDone(m_omx_encoder.GetComponent(), omx_buffer);
break;
}
}
Expand All @@ -1397,12 +1409,15 @@ bool COMXImageEnc::Encode(unsigned char *buffer, int size, unsigned width, unsig

omx_err = m_omx_encoder.FillThisBuffer(m_encoded_buffer);
if(omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s m_omx_encoder.FillThisBuffer result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_encoder.DecoderFillBufferDone(m_omx_encoder.GetComponent(), m_encoded_buffer);
return false;

}
omx_err = m_omx_encoder.WaitForOutputDone(1000);
if(omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s m_omx_resize.WaitForOutputDone result(0x%x)\n", CLASSNAME, __func__, omx_err);
CLog::Log(LOGERROR, "%s::%s m_omx_encoder.WaitForOutputDone result(0x%x)\n", CLASSNAME, __func__, omx_err);
return false;
}

Expand Down Expand Up @@ -1445,6 +1460,7 @@ bool COMXImageEnc::CreateThumbnailFromSurface(unsigned char* buffer, unsigned in
return true;
}

m_success = true;
return false;
}

Expand Down Expand Up @@ -1842,6 +1858,7 @@ bool COMXImageReEnc::ReEncode(COMXImageFile &srcFile, unsigned int maxWidth, uns
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s %s OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, srcFile.GetFilename(), omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return false;
}
}
Expand Down Expand Up @@ -1880,6 +1897,7 @@ bool COMXImageReEnc::ReEncode(COMXImageFile &srcFile, unsigned int maxWidth, uns
if(omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s %s FillThisBuffer() failed (%x)\n", CLASSNAME, __func__, srcFile.GetFilename(), omx_err);
m_omx_encoder.DecoderFillBufferDone(m_omx_encoder.GetComponent(), m_encoded_buffer);
return false;
}
}
Expand Down Expand Up @@ -2199,6 +2217,7 @@ bool COMXTexture::Decode(const uint8_t *demuxer_content, unsigned demuxer_bytes,
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s - m_omx_decoder.OMX_EmptyThisBuffer (%x)", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return false;
}
}
Expand Down Expand Up @@ -2248,6 +2267,7 @@ bool COMXTexture::Decode(const uint8_t *demuxer_content, unsigned demuxer_bytes,
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s error m_omx_egl_render.FillThisBuffer (%x)", CLASSNAME, __func__, omx_err);
m_omx_egl_render.DecoderFillBufferDone(m_omx_egl_render.GetComponent(), m_egl_buffer);
return false;
}

Expand Down
38 changes: 11 additions & 27 deletions xbmc/cores/omxplayer/OMXVideo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,7 @@ bool COMXVideo::SendDecoderConfig()
}

omx_buffer->nOffset = 0;
omx_buffer->nFilledLen = m_extrasize;
if(omx_buffer->nFilledLen > omx_buffer->nAllocLen)
{
CLog::Log(LOGERROR, "%s::%s - omx_buffer->nFilledLen > omx_buffer->nAllocLen", CLASSNAME, __func__);
return false;
}
omx_buffer->nFilledLen = std::min((OMX_U32)m_extrasize, omx_buffer->nAllocLen);

memset((unsigned char *)omx_buffer->pBuffer, 0x0, omx_buffer->nAllocLen);
memcpy((unsigned char *)omx_buffer->pBuffer, m_extradata, omx_buffer->nFilledLen);
Expand All @@ -124,6 +119,7 @@ bool COMXVideo::SendDecoderConfig()
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return false;
}
}
Expand Down Expand Up @@ -654,8 +650,7 @@ bool COMXVideo::Open(CDVDStreamInfo &hints, OMXClock *clock, EDEINTERLACEMODE de
return false;
}

if(!SendDecoderConfig())
return false;
SendDecoderConfig();

m_is_open = true;
m_drop_state = false;
Expand Down Expand Up @@ -799,7 +794,7 @@ int COMXVideo::Decode(uint8_t *pData, int iSize, double pts)
omx_buffer->nFlags |= OMX_BUFFERFLAG_TIME_UNKNOWN;

omx_buffer->nTimeStamp = ToOMXTime((uint64_t)(pts == DVD_NOPTS_VALUE) ? 0 : pts);
omx_buffer->nFilledLen = (demuxer_bytes > omx_buffer->nAllocLen) ? omx_buffer->nAllocLen : demuxer_bytes;
omx_buffer->nFilledLen = std::min((OMX_U32)demuxer_bytes, omx_buffer->nAllocLen);
memcpy(omx_buffer->pBuffer, demuxer_content, omx_buffer->nFilledLen);

demuxer_bytes -= omx_buffer->nFilledLen;
Expand All @@ -808,26 +803,14 @@ int COMXVideo::Decode(uint8_t *pData, int iSize, double pts)
if(demuxer_bytes == 0)
omx_buffer->nFlags |= OMX_BUFFERFLAG_ENDOFFRAME;

int nRetry = 0;
while(true)
omx_err = m_omx_decoder.EmptyThisBuffer(omx_buffer);
if (omx_err != OMX_ErrorNone)
{
omx_err = m_omx_decoder.EmptyThisBuffer(omx_buffer);
if (omx_err == OMX_ErrorNone)
{
//CLog::Log(LOGINFO, "VideD: dts:%.0f pts:%.0f size:%d)\n", dts, pts, iSize);
break;
}
else
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
nRetry++;
}
if(nRetry == 5)
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() finally failed\n", CLASSNAME, __func__);
return false;
}
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return false;
}
//CLog::Log(LOGINFO, "VideD: dts:%.0f pts:%.0f size:%d)\n", dts, pts, iSize);

omx_err = m_omx_decoder.WaitForEvent(OMX_EventPortSettingsChanged, 0);
if (omx_err == OMX_ErrorNone)
Expand Down Expand Up @@ -946,6 +929,7 @@ void COMXVideo::SubmitEOS()
if (omx_err != OMX_ErrorNone)
{
CLog::Log(LOGERROR, "%s::%s - OMX_EmptyThisBuffer() failed with result(0x%x)\n", CLASSNAME, __func__, omx_err);
m_omx_decoder.DecoderEmptyBufferDone(m_omx_decoder.GetComponent(), omx_buffer);
return;
}
CLog::Log(LOGINFO, "%s::%s", CLASSNAME, __func__);
Expand Down

0 comments on commit 97e656c

Please sign in to comment.