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

[Lynne]

On 15/06/2024 18:05, Tong Wu 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.
> 
> Sure. Thank you.
> 
> -Tong
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

Sorry for the delay. Patchset pushed.
I'll post the Vulkan encode patches which use this shortly.
Thanks for all the hard work you put into it.
-------------- 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/20240702/906af311/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/20240702/906af311/attachment.sig>