>From: Lynne <d...@lynne.ee>
>Sent: Monday, May 27, 2024 10:04 AM
>To: Wu, Tong1 <tong1...@intel.com>; FFmpeg development discussions and
>patches <ffmpeg-devel@ffmpeg.org>
>Subject: Re: [FFmpeg-devel] [PATCH v11 07/14] avcodec/vaapi_encode: extract
>the init and close function to base layer
>
>On 27/05/2024 02:35, Wu, Tong1 wrote:
>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
>Lynne
>>> via ffmpeg-devel
>>> Sent: Saturday, May 25, 2024 10:07 PM
>>> To: ffmpeg-devel@ffmpeg.org
>>> Cc: Lynne <d...@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....@ffmpeg.org wrote:
>>>> From: Tong Wu <tong1...@intel.com>
>>>>
>>>> Related parameters such as device context, frame context are also moved
>>>> to base layer.
>>>>
>>>> Signed-off-by: Tong Wu <tong1...@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?
>
>Yup. My point is that FFHWBaseEncodeContext doesn't become the primary
>context that must be used, but a separate component other users can use
>standalone.
>
>> 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.
>
>Just save a pointer to avctx on init in FFHWBaseEncodeContext. That's
>what most of our code does.
>
>> 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.
>
>No, the callbacks should stay as-is. Users need to retrieve their own
>context via avctx. That's also a reason why you should keep a pointer to
>avctx on init.
>
I've updated v12 and made this change with a separate(15th) patch. Please see
if that is what you wanted. Thank you.
-Tong
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
