Dear Experts,
If ALT_BITSTREAM_ALLOC flag is set. under certain conditions the encoder may attempt to expand previously allocated bitstream buffer for a particular slice (umc_h264_core_enc_tmpl.cpp.h). The memory is released by making a "free" call, then re-allocated. There seems to be a problem with this though because as far as I understand, the pointer being freed may not be the one previously obtained by making an "alloc" call: H264CoreEncoder_Init function (umc_h264_gen_enc_tmpl.cpp.h) allocates a single large buffer and assigns pointers into this large buffer for each slice. Am I correct in believing that this code may cause heap corruption?
The encoder crash I was experiencing seemed to go away after I disabled ALT_BITSTREAM_ALLOC and allowed a bigger bitstream buffer, but, on the other hand, my attempt to keep the resizing code and add changes such that individual buffers are allocated for each slice didn't work well.
Thanks!
連結已複製
3 回應
Good day.
Yes it seems that this code doesn't work properly.
I made some modifications and tested it with small buffer setting, works fine now.
You need to separate buffer allocations for each slice and update destructor:
[cpp]
template
Status H264CoreEncoder_Init(void* state, BaseCodecParams* init, MemoryAllocator* pMemAlloc)
{
...
//Ipp32u bsSize = core_enc->m_PaddedSize.width; // small buffer test
Ipp32u bsSize = core_enc->m_PaddedSize.width * core_enc->m_PaddedSize.height * sizeof(PIXTYPE);
bsSize += (bsSize >> 1) + 4096;
// TBD: see if buffer size can be reduced
core_enc->m_pbitstreams = (H264BsReal**)H264_Malloc(numOfSliceEncs * sizeof(H264BsReal*));
if(!core_enc->m_pbitstreams)
return UMC_ERR_ALLOC;
for (i = 0; i < numOfSliceEncs; i++)
{
Ipp8u* pTmpBuffer = (Ipp8u*)H264_Malloc(bsSize + DATA_ALIGN);
if(!pTmpBuffer)
return UMC_ERR_ALLOC;
core_enc->m_pbitstreams = (H264BsReal*)H264_Malloc(sizeof(H264BsReal));
if (!core_enc->m_pbitstreams)
return UMC_ERR_ALLOC;
H264BsReal_Create(core_enc->m_pbitstreams, pTmpBuffer, bsSize, core_enc->m_params.chroma_format_idc, status);
if (status != UMC_OK)
return status;
core_enc->m_Slices.m_pbitstream = (H264BsBase*)core_enc->m_pbitstreams;
}
core_enc->m_bs1 = core_enc->m_pbitstreams[0]; // core_enc->m_bs1 is the main stream.
...
}
...
template
Status H264CoreEncoder_Close(void* state)
{
...
if (core_enc->m_pbitstreams)
{
Ipp32s i;
for (i = 0; i < core_enc->m_params.num_slices*((core_enc->m_params.coding_type == 1) + 1); i++)
{ //TODO fix for PicAFF/AFRM
if (core_enc->m_pbitstreams)
{
H264_Free(core_enc->m_pbitstreams->m_base.m_pbsBase);
H264_Free(core_enc->m_pbitstreams);
core_enc->m_pbitstreams = NULL;
}
}
H264_Free(core_enc->m_pbitstreams);
core_enc->m_pbitstreams = NULL;
...
}
[/cpp]
And fix data shifts in reallocator:
[cpp]
template
Status H264CoreEncoder_Compress_Slice(void* state, H264Slice *curr_slice, bool is_first_mb)
{
...
#ifdef ALT_BITSTREAM_ALLOC
//Expand buffer if it is nearly full
Ipp32u bytesInBuffer = H264BsBase_GetBsSize(&(pBitstream->m_base));
if (bytesInBuffer >= 3 * (pBitstream->m_base.m_maxBsSize >> 2))
{
pBitstream->m_base.m_maxBsSize <<= 1;
size_t iRBSPShift = pBitstream->m_pbsRBSPBase - pBitstream->m_base.m_pbsBase;
Ipp8u* tmpBitstreamBuf = (Ipp8u*)H264_Malloc(pBitstream->m_base.m_maxBsSize + DATA_ALIGN);
ippsSet_8u(0, (Ipp8u*)tmpBitstreamBuf, pBitstream->m_base.m_maxBsSize);
ippsCopy_8u(pBitstream->m_base.m_pbsBase,tmpBitstreamBuf,bytesInBuffer);
H264_Free(pBitstream->m_base.m_pbsBase);
pBitstream->m_base.m_pbsBase = tmpBitstreamBuf;
pBitstream->m_pbsRBSPBase = tmpBitstreamBuf + iRBSPShift;
pBitstream->m_base.m_pbs = pBitstream->m_base.m_pbsBase + ((pBitstream->m_base.m_bitOffset)?bytesInBuffer - 1:bytesInBuffer);
}
#endif
...
}
[/cpp]
Dear Expert,
Thank you for your response. I'd like to suggest a couple more fixes in addition to the changes you posted.
1) Original code used to align the allocated pointer before using it. I made changes, based on your modifications, to align the pointer (in Init() and Compress_Slice() methods). I also redefined m_pAllocEncoderInst to serve as an array of unaligned pointers as returned by malloc() so that free() can be called on the pointers once they are no longer needed when reallocating a buffer or releasing as a part of Close().
2) In the version of the code I am working with, there are pointers to the bitstream buffer initialized prior to the point where it may be decided that the buffer needs to be expanded. I made changes to the Compress_Slice() method to ensure that the pointers are reassigned appropriately in relation to the new (expanded) buffer if the expansion occurs.
After making the fixes mentioned above (especially the latter) I seem to be no longer experiencing the issue with the encoder crashing. I'm attaching the clip I was using to reproduce the issue, just in case if someone feels like experimenting with it.
Thanks!
