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

[Lynne]

On 12/06/2024 10:44, Wu, Tong1 wrote:
>> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xA2FEA5F03F034464.asc
Type: application/pgp-keys
Size: 624 bytes
Desc: OpenPGP public key
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240614/e76cb1f1/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240614/e76cb1f1/attachment.sig>