MediaSDK icon indicating copy to clipboard operation
MediaSDK copied to clipboard

Samples: Current implementations of CopyBitstream2 might create issue in CH264FrameReader

Open CarlG12 opened this issue 4 years ago • 1 comments

The file sample_utils.cpp defines a function CopyBitstream2 that is wrongly implemented. This method is used by the CH264FrameReader class and consumed in decoding pipeline.

First of all, is not clear if the method CopyBitstream2 shall:

  • Create an exact copy of bitstream buffer "src" in bitstream buffer "dest".
  • Append bitstream buffer "src" in bitstream buffer "dest"

This is the current implementation:

mfxStatus CopyBitstream2(mfxBitstream *dest, mfxBitstream *src)
{
    if (!dest || !src)
        return MFX_ERR_NULL_PTR;

    if (!dest->DataLength)
    {
        dest->DataOffset = 0;
    }
    else
    {
        memmove(dest->Data, dest->Data + dest->DataOffset, dest->DataLength);
        dest->DataOffset = 0;
    }

    if (src->DataLength > dest->MaxLength - dest->DataLength - dest->DataOffset)
        return MFX_ERR_NOT_ENOUGH_BUFFER;

    MSDK_MEMCPY_BITSTREAM(*dest, dest->DataOffset, src->Data, src->DataLength);
    dest->DataLength = src->DataLength;

    dest->DataFlag = src->DataFlag;

    //common Extended buffer will be for src and dest bit streams
    dest->EncryptedData = src->EncryptedData;

    return MFX_ERR_NONE;
}

1. If the goal is to create an exact copy of "src" in "dest": A) This method is inefficient because the unwanted memmove operation B) Can returns wrongly the error "MFX_ERR_NOT_ENOUGH_BUFFER" if the initial value of dest->DataLenght differs than zero.

2. If the goal is to append src in dest: A) MSDK_MEMCPY_BITSTREAM does not reach the goal because the destination offset parameter shall be "dest->DataLength" instead of "dest->DataOffset" that is always set to zero. B) dest->DataLength is not adjusted correctly. It is currently replaced by src->DataLength instead of being added with.

CarlG12 avatar Sep 10 '21 02:09 CarlG12

@onabiull and @aukhina , depending of the objective of the method CopyBitstream2, I think that this code need a fix. Please, ask me clarification if it's not clear. Thanks

CarlG12 avatar Jan 12 '22 23:01 CarlG12