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

[Wu, Tong1]

>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?

>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