[FFmpeg-devel] [PATCH v11 07/14] avcodec/vaapi_encode: extract the init and close function to base layer

[Wu, Tong1]

>From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Lynne
>via ffmpeg-devel
>Sent: Saturday, May 25, 2024 10:07 PM
>To: ffmpeg-devel at ffmpeg.org
>Cc: Lynne <dev at lynne.ee>
>Subject: Re: [FFmpeg-devel] [PATCH v11 07/14] avcodec/vaapi_encode: extract
>the init and close function to base layer
>
>On 25/05/2024 12:30, tong1.wu-at-intel.com at ffmpeg.org wrote:
>> From: Tong Wu <tong1.wu at intel.com>
>>
>> Related parameters such as device context, frame context are also moved
>> to base layer.
>>
>> Signed-off-by: Tong Wu <tong1.wu at intel.com>
>> ---
>>   libavcodec/hw_base_encode.c     | 49 ++++++++++++++++++
>>   libavcodec/hw_base_encode.h     | 17 +++++++
>>   libavcodec/vaapi_encode.c       | 90 +++++++++++----------------------
>>   libavcodec/vaapi_encode.h       | 10 ----
>>   libavcodec/vaapi_encode_av1.c   |  2 +-
>>   libavcodec/vaapi_encode_h264.c  |  2 +-
>>   libavcodec/vaapi_encode_h265.c  |  2 +-
>>   libavcodec/vaapi_encode_mjpeg.c |  6 ++-
>>   8 files changed, 102 insertions(+), 76 deletions(-)
>>
>> diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
>> index 16afaa37be..c4789380b6 100644
>> --- a/libavcodec/hw_base_encode.c
>> +++ b/libavcodec/hw_base_encode.c
>> @@ -592,3 +592,52 @@ end:
>>
>>       return 0;
>>   }
>> +
>> +int ff_hw_base_encode_init(AVCodecContext *avctx)
>> +{
>> +    FFHWBaseEncodeContext *ctx = avctx->priv_data;
>
>This is the issue I was talking about, this requires that
>FFHWBaseEncodeContext is always the main context.
>
>Could you change it so everything takes FFHWBaseEncodeContext as an
>argument, rather than AVCodecContext (apart from where the function
>absolutely must read some data from it)?

I'm trying to understand it more.

In ff_hw_base_encode_init we also have the following code:

    if (!avctx->hw_frames_ctx) {
        av_log(avctx, AV_LOG_ERROR, "A hardware frames reference is "
               "required to associate the encoding device.\n");
        return AVERROR(EINVAL);
    }

In this scenario we still need avctx right? So maybe this is the "must read data from it" situation and we keep AVCodecContext as the main context? 

Plus I have this av_log concern which is there's indeed some function that the only use of avctx is its av_log's context. Do you think I should instead use FFHWBaseEncodeContext as the main context and pass it to av_log for this situation while keeping AVCodecContext as the main context for other functions which actually read from AVCodecContext. That might lead to different av_log context in the same file.

Do you think the callbacks in FFHWEncodePictureOperation should be changed too? Or only all the functions in hw_base_encode.c should be concerned. Thank you.

-Tong