[FFmpeg-devel] [PATCH] avcodec/nvdec: don't free NVDECContext->bitstream

[Timo Rothenpieler]

On 07.02.2024 00:05, James Almer wrote:
> On 2/6/2024 8:00 PM, Timo Rothenpieler wrote:
>> On 06.02.2024 23:57, James Almer wrote:
>>> Ensure all hwaccels that allocate a buffer use 
>>> NVDECContext->bitstream_internal.
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>   libavcodec/nvdec.c      | 2 +-
>>>   libavcodec/nvdec_h264.c | 4 ++--
>>>   libavcodec/nvdec_hevc.c | 4 ++--
>>>   3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavcodec/nvdec.c b/libavcodec/nvdec.c
>>> index 27be644356..d13b790632 100644
>>> --- a/libavcodec/nvdec.c
>>> +++ b/libavcodec/nvdec.c
>>> @@ -259,8 +259,8 @@ int ff_nvdec_decode_uninit(AVCodecContext *avctx)
>>>   {
>>>       NVDECContext *ctx = avctx->internal->hwaccel_priv_data;
>>> -    av_freep(&ctx->bitstream);
>>>       av_freep(&ctx->bitstream_internal);
>>> +    ctx->bitstream           = NULL;
>>>       ctx->bitstream_len       = 0;
>>>       ctx->bitstream_allocated = 0;
>>> diff --git a/libavcodec/nvdec_h264.c b/libavcodec/nvdec_h264.c
>>> index f022619b64..8c72d5f4f7 100644
>>> --- a/libavcodec/nvdec_h264.c
>>> +++ b/libavcodec/nvdec_h264.c
>>> @@ -138,11 +138,11 @@ static int 
>>> nvdec_h264_decode_slice(AVCodecContext *avctx, const uint8_t *buffer,
>>>       const H264SliceContext *sl = &h->slice_ctx[0];
>>>       void *tmp;
>>> -    tmp = av_fast_realloc(ctx->bitstream, &ctx->bitstream_allocated,
>>> +    tmp = av_fast_realloc(ctx->bitstream_internal, 
>>> &ctx->bitstream_allocated,
>>>                             ctx->bitstream_len + size + 3);
>>>       if (!tmp)
>>>           return AVERROR(ENOMEM);
>>> -    ctx->bitstream = tmp;
>>> +    ctx->bitstream = ctx->bitstream_internal = tmp;
>>>       tmp = av_fast_realloc(ctx->slice_offsets, 
>>> &ctx->slice_offsets_allocated,
>>>                             (ctx->nb_slices + 1) * 
>>> sizeof(*ctx->slice_offsets));
>>> diff --git a/libavcodec/nvdec_hevc.c b/libavcodec/nvdec_hevc.c
>>> index b83d5edcf9..25319a1328 100644
>>> --- a/libavcodec/nvdec_hevc.c
>>> +++ b/libavcodec/nvdec_hevc.c
>>> @@ -274,11 +274,11 @@ static int 
>>> nvdec_hevc_decode_slice(AVCodecContext *avctx, const uint8_t *buffer,
>>>       NVDECContext *ctx = avctx->internal->hwaccel_priv_data;
>>>       void *tmp;
>>> -    tmp = av_fast_realloc(ctx->bitstream, &ctx->bitstream_allocated,
>>> +    tmp = av_fast_realloc(ctx->bitstream_internal, 
>>> &ctx->bitstream_allocated,
>>>                             ctx->bitstream_len + size + 3);
>>>       if (!tmp)
>>>           return AVERROR(ENOMEM);
>>> -    ctx->bitstream = tmp;
>>> +    ctx->bitstream = ctx->bitstream_internal = tmp;
>>>       tmp = av_fast_realloc(ctx->slice_offsets, 
>>> &ctx->slice_offsets_allocated,
>>>                             (ctx->nb_slices + 1) * 
>>> sizeof(*ctx->slice_offsets));
>>
>> bitstream_internal was added for the av1 slice reconstructions.
>> Probably for the exact reason to avoid this confusion.
>>
>> Looking at this some more... With this patch in place, there should 
>> never be a situation in which we want to free/freep ctx->bitstream?
>> So the freep() on it should probably be removed alongside, and replace 
>> with just setting it to NULL.
> 
> That's what i did above, in the libavcodec/nvdec.c chunk.

Ah, I somehow missed that!