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

[Timo Rothenpieler]

On 07.02.2024 00:00, 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.

Likewise, the entire need for the _simple_end_frame function is gone.
NULL'ing the bitstream can just be done in the normal one, and all 
decoders can use it.