Re: [FFmpeg-devel] [PATCH v4 15/16] FFHWAccel: add buffer_ref argument to start_frame

[Lynne <dev@lynne.ee>]

On 13/03/2025 23:08, Andreas Rheinhardt wrote:
> Lynne:
>> This commit adds a reference to the buffer as an argument to
>> start_frame, and adapts all existing code.
>>
>> This allows for asynchronous hardware accelerators to skip
>> copying packet data by referencing it.
>> ---
>>   
>> diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
>> index da8fdc5935..20ef821819 100644
>> --- a/libavcodec/hevc/hevcdec.c
>> +++ b/libavcodec/hevc/hevcdec.c
>> @@ -3401,7 +3401,10 @@ static int hevc_frame_start(HEVCContext *s, HEVCLayerContext *l,
>>           goto fail;
>>   
>>       if (s->avctx->hwaccel) {
>> -        ret = FF_HW_CALL(s->avctx, start_frame, NULL, 0);
>> +        AVCodecInternal *avci = s->avctx->internal;
>> +        AVPacket *avpkt = avci->in_pkt;
>> +        ret = FF_HW_CALL(s->avctx, start_frame,
>> +                         avpkt->buf, NULL, 0);
>>           if (ret < 0)
>>               goto fail;
>>       }
>> diff --git a/libavcodec/hwaccel_internal.h b/libavcodec/hwaccel_internal.h
>> index 77df4e0904..4eb74f0b34 100644
>> --- a/libavcodec/hwaccel_internal.h
>> +++ b/libavcodec/hwaccel_internal.h
>> @@ -52,11 +52,13 @@ typedef struct FFHWAccel {
>>        * Otherwise, this means the whole frame is available at this point.
>>        *
>>        * @param avctx the codec context
>> +     * @param buf_ref the frame data buffer reference (optional)
>>        * @param buf the frame data buffer base
>>        * @param buf_size the size of the frame in bytes
>>        * @return zero if successful, a negative value otherwise
>>        */
>> -    int (*start_frame)(AVCodecContext *avctx, const uint8_t *buf, uint32_t buf_size);
>> +    int (*start_frame)(AVCodecContext *avctx, AVBufferRef *buf_ref,
> 
> const AVBufferRef*, we are not passing ownership

Done locally.

> Why did you not make this the last parameter (given that only very few
> hwaccels will access it)?

It made more sense to have it upfront as it backs up the data given.
I can change it, but I'd rather not, unless you have a strong opinion.

>> +                       const uint8_t *buf, uint32_t buf_size);
>>   
>>       /**
>>        * Callback for parameter data (SPS/PPS/VPS etc).
>> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
>> index bd1b502e50..f3d940671e 100644
>> --- a/libavcodec/mjpegdec.c
>> +++ b/libavcodec/mjpegdec.c
>> @@ -808,7 +808,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>>           if (!s->hwaccel_picture_private)
>>               return AVERROR(ENOMEM);
>>   
>> -        ret = hwaccel->start_frame(s->avctx, s->raw_image_buffer,
>> +        ret = hwaccel->start_frame(s->avctx, NULL, s->raw_image_buffer,
>>                                      s->raw_image_buffer_size);
>>           if (ret < 0)
>>               return ret;
>> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>> index ffe0710470..c21f220680 100644
>> --- a/libavcodec/mpeg12dec.c
>> +++ b/libavcodec/mpeg12dec.c
>> @@ -1361,7 +1361,7 @@ static int mpeg_field_start(Mpeg1Context *s1, const uint8_t *buf, int buf_size)
>>       }
>>   
>>       if (avctx->hwaccel) {
>> -        if ((ret = FF_HW_CALL(avctx, start_frame, buf, buf_size)) < 0)
>> +        if ((ret = FF_HW_CALL(avctx, start_frame, NULL, buf, buf_size)) < 0)
>>               return ret;
>>       } else if (s->codec_tag == MKTAG('V', 'C', 'R', '2')) {
>>           // Exchange UV
>> diff --git a/libavcodec/nvdec_av1.c b/libavcodec/nvdec_av1.c
>> index 6b408edb87..9b9be702d5 100644
>> --- a/libavcodec/nvdec_vp9.c
>> +++ b/libavcodec/nvdec_vp9.c
>> @@ -29,7 +29,8 @@
>>   #include "internal.h"
>>   #include "vp9shared.h"
>>   
>> -static int nvdec_vp9_start_frame(AVCodecContext *avctx, const uint8_t *buffer, uint32_t size)
>> +static int nvdec_vp9_start_frame(AVCodecContext *avctx, AVBufferRef *buffer_ref,
>> +                                 const uint8_t *buffer, uint32_t size)
>>   {
>>       VP9SharedContext *h = avctx->priv_data;
>>       const AVPixFmtDescriptor *pixdesc = av_pix_fmt_desc_get(avctx->sw_pix_fmt);
>> diff --git a/libavcodec/proresdec.c b/libavcodec/proresdec.c
>> index 01caa611a0..edc46e1442 100644
>> --- a/libavcodec/proresdec.c
>> +++ b/libavcodec/proresdec.c
>> @@ -793,7 +793,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
>>   
>>       if (HWACCEL_MAX && avctx->hwaccel) {
>>           const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
>> -        ret = hwaccel->start_frame(avctx, NULL, 0);
>> +        ret = hwaccel->start_frame(avctx, avpkt->buf, NULL, 0);
> 
> Passing an AVBufferRef for NULL data seems fishy.

I misinterpreted what NULL means.
Changed this to be NULL.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".