[FFmpeg-devel] [PATCH v13 06/15] avcodec/vaapi_encode: move the dpb logic from VAAPI to base layer

[Wu Tong]

>From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Tong
>Wu
>Sent: 2024年6月16日 0:06
>To: FFmpeg development discussions and patches <ffmpeg-
>devel at ffmpeg.org>
>Cc: Lynne <dev at lynne.ee>
>Subject: Re: [FFmpeg-devel] [PATCH v13 06/15] avcodec/vaapi_encode: move
>the dpb logic from VAAPI to base layer
>
>> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> >> Lynne via ffmpeg-devel
>> >> Sent: Monday, June 10, 2024 10:01 AM
>> >> To: FFmpeg development discussions and patches <ffmpeg-
>> >> devel at ffmpeg.org>
>> >> Cc: Lynne <dev at lynne.ee>
>> >> Subject: Re: [FFmpeg-devel] [PATCH v13 06/15] avcodec/vaapi_encode:
>> >> move the dpb logic from VAAPI to base layer
>> >>
>> >> On 07/06/2024 18:48, Lynne wrote:
>> >>> On 07/06/2024 17:22, Wu, Tong1 wrote:
>> >>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf
>> >>>>> Of
>> >> Lynne
>> >>>>> via ffmpeg-devel
>> >>>>> Sent: Friday, June 7, 2024 11:10 PM
>> >>>>> To: ffmpeg-devel at ffmpeg.org
>> >>>>> Cc: Lynne <dev at lynne.ee>
>> >>>>> Subject: Re: [FFmpeg-devel] [PATCH v13 06/15]
>> avcodec/vaapi_encode:
>> >> move
>> >>>>> the dpb logic from VAAPI to base layer
>> >>>>>
>> >>>>> On 03/06/2024 11:18, tong1.wu-at-intel.com at ffmpeg.org wrote:
>> >>>>>> From: Tong Wu <tong1.wu at intel.com>
>> >>>>>>
>> >>>>>> Move receive_packet function to base. This requires adding
>> >>>>>> *alloc, *issue, *output, *free as hardware callbacks.
>> >>>>>> HWBaseEncodePicture is introduced as the base layer structure.
>> >>>>>> The related parameters in VAAPIEncodeContext are also extracted
>> >>>>>> to HWBaseEncodeContext. Then
>> >> DPB
>> >>>>>> management logic can be fully extracted to base layer as-is.
>> >>>>>>
>> >>>>>> Signed-off-by: Tong Wu <tong1.wu at intel.com>
>> >>>>>> ---
>> >>>>>>     libavcodec/Makefile             |   2 +-
>> >>>>>>     libavcodec/hw_base_encode.c     | 594
>> >>>>>> ++++++++++++++++++++++++
>> >>>>>>     libavcodec/hw_base_encode.h     | 124 +++++
>> >>>>>>     libavcodec/vaapi_encode.c       | 793 +++++
>> >>>>>> ---------------------------
>> >>>>>>     libavcodec/vaapi_encode.h       | 102 +---
>> >>>>>>     libavcodec/vaapi_encode_av1.c   |  35 +-
>> >>>>>>     libavcodec/vaapi_encode_h264.c  |  84 ++--
>> >>>>>>     libavcodec/vaapi_encode_h265.c  |  53 ++-
>> >>>>>>     libavcodec/vaapi_encode_mjpeg.c |  13 +-
>> >>>>>>     libavcodec/vaapi_encode_mpeg2.c |  33 +-
>> >>>>>>     libavcodec/vaapi_encode_vp8.c   |  18 +-
>> >>>>>>     libavcodec/vaapi_encode_vp9.c   |  24 +-
>> >>>>>>     12 files changed, 985 insertions(+), 890 deletions(-)
>> >>>>>>     create mode 100644 libavcodec/hw_base_encode.c
>> >>>>>
>> >>>>> This patch doesn't apply,
>> >>>>>
>> >>>>> error: sha1 information is lacking or useless (libavcodec/
>> >>>>> hw_base_encode.c).
>> >>>>> error: could not build fake ancestor
>> >>>>>
>> >>>>> Could you resent the patchset or link me a repo so I can work with it?
>> >>>>
>> >>>> https://github.com/intel-media-ci/ffmpeg/pull/689 This is the
>> >>>> same as
>> >>>> v13 please have a try.
>> >>>
>> >>> That worked, thanks.
>> >>
>> >> I don't think the behaviour is correct when the encoding length is
>> >> less than the decode delay. In my old Vulkan code, I had this piece
>> >> of code in the initialization function:
>> >>
>> >>> if (!src) {
>> >>>      ctx->end_of_stream = 1;
>> >>>      /* Fix timestamps if we hit end-of-stream before the initial
>> >>>       * decode delay has elapsed. */
>> >>>      if (ctx->input_order < ctx->decode_delay)
>> >>>          ctx->dts_pts_diff = ctx->pic_end->pts - ctx->first_pts;
>> >>>      return AVERROR_EOF;
>> >>> }
>> >>
>> >> I think a flush function should be added, to be called by each
>> >> encoder, to make sure the timestamps remain correct.
>> >>
>> >
>> > For the current patch set, this piece is in
>> > hw_base_encode_send_frame and
>> works well for vaapi and d3d12 except when the encoding length is
>> equal to the decode delay, which I'll sent a fix later. Do you mean
>> Vulkan cannot integrate into this part and we have to make a callback for it?
>>
>> No, I was just curious. Fair enough, it can be implemented in a later patch.
>>
>> >
>> >> Also, the D3D12VA structures need an FF prefix, e.g.
>> >> D3D12VAEncodeContext -> FFD3D12VAEncodeContext.
>> >
>> > The current VAAPIEncodeContext has existed for a long time. Does it
>> > have
>> any difference for D3D12VAEncodeContext? I mean both
>> VAAPIEncodeContext and D3D12VAEncodeContext are parallel and only
>> referenced in vaapi_encode_*.c (d3d12va_encode_*.c).
>> >
>> > Thanks,
>> > Tong
>>
>> I'm finishing up on the Vulkan test implementation, I'll see to
>> pushing this patch over the weekend.
>
>Sure. Thank you.
>

Kindly ping. Could we push this patch set?

Thanks,
Tong