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

[Lynne via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 4970 bytes --]

On 15/06/2024 18:05, Tong Wu wrote:
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@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@ffmpeg.org>
>>>> Cc: Lynne <dev@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@ffmpeg.org> On Behalf Of
>>>> Lynne
>>>>>>> via ffmpeg-devel
>>>>>>> Sent: Friday, June 7, 2024 11:10 PM
>>>>>>> To: ffmpeg-devel@ffmpeg.org
>>>>>>> Cc: Lynne <dev@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@ffmpeg.org wrote:
>>>>>>>> From: Tong Wu <tong1.wu@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@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@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.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".