[FFmpeg-devel] [PATCH] vaapi_encode_h264: Only set pic_order_cnt_type to 0 with B-frames

[FFmpeg-devel] [PATCH] vaapi_encode_h264: Only set pic_order_cnt_type to 0 with B-frames

[David Rosca]

---
 libavcodec/vaapi_encode_h264.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index dd17be2..d6926c4 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -350,7 +350,7 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
     sps->chroma_format_idc    = 1;
 
     sps->log2_max_frame_num_minus4 = 4;
-    sps->pic_order_cnt_type        = 0;
+    sps->pic_order_cnt_type        = ctx->max_b_depth ? 0 : 2;
     sps->log2_max_pic_order_cnt_lsb_minus4 = 4;
 
     sps->max_num_ref_frames = priv->dpb_frames;
-- 
2.39.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] vaapi_encode_h264: Only set pic_order_cnt_type to 0 with B-frames

[Xiang, Haihao]

On Do, 2022-12-29 at 22:20 +0100, David Rosca wrote:
> ---
>  libavcodec/vaapi_encode_h264.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index dd17be2..d6926c4 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -350,7 +350,7 @@ static int
> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>      sps->chroma_format_idc    = 1;
>  
>      sps->log2_max_frame_num_minus4 = 4;
> -    sps->pic_order_cnt_type        = 0;
> +    sps->pic_order_cnt_type        = ctx->max_b_depth ? 0 : 2;


pic_order_cnt_type (0) should work for ctx->max_b_depth == 0. If 2 is preferred
for your vaapi driver, it would be better to query the capability from the
driver, like as what commit 9f02e033875185409c861846f209b04a3be339d2 did. 

Thanks
Haihao

>      sps->log2_max_pic_order_cnt_lsb_minus4 = 4;
>  
>      sps->max_num_ref_frames = priv->dpb_frames;
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] vaapi_encode_h264: Only set pic_order_cnt_type to 0 with B-frames

[David Rosca]

On Mon, Jan 9, 2023 at 3:22 AM Xiang, Haihao <haihao.xiang@intel.com> wrote:
>
> On Do, 2022-12-29 at 22:20 +0100, David Rosca wrote:
> > ---
> >  libavcodec/vaapi_encode_h264.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> > index dd17be2..d6926c4 100644
> > --- a/libavcodec/vaapi_encode_h264.c
> > +++ b/libavcodec/vaapi_encode_h264.c
> > @@ -350,7 +350,7 @@ static int
> > vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
> >      sps->chroma_format_idc    = 1;
> >
> >      sps->log2_max_frame_num_minus4 = 4;
> > -    sps->pic_order_cnt_type        = 0;
> > +    sps->pic_order_cnt_type        = ctx->max_b_depth ? 0 : 2;
>
>
> pic_order_cnt_type (0) should work for ctx->max_b_depth == 0. If 2 is preferred
> for your vaapi driver, it would be better to query the capability from the
> driver, like as what commit 9f02e033875185409c861846f209b04a3be339d2 did.

It's not about an encoder, but rather about decoder. Some decoders
(namely the Snapdragon HW decoder)
will buffer frames when pic_order_cnt_type == 0 (in case the frames
will be reordered?) which results
in undesired increased latency. Setting pic_order_cnt_type to 2 will
fix this problem, and it is also what
libx264 does [0].

Regards,
David

[0] https://code.videolan.org/videolan/x264/-/blob/master/encoder/set.c#L173
>
> Thanks
> Haihao
>
> >      sps->log2_max_pic_order_cnt_lsb_minus4 = 4;
> >
> >      sps->max_num_ref_frames = priv->dpb_frames;
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] vaapi_encode_h264: Only set pic_order_cnt_type to 0 with B-frames

[Mark Thompson]

On 09/01/2023 07:37, David Rosca wrote:
> On Mon, Jan 9, 2023 at 3:22 AM Xiang, Haihao <haihao.xiang@intel.com> wrote:
>>
>> On Do, 2022-12-29 at 22:20 +0100, David Rosca wrote:
>>> ---
>>>   libavcodec/vaapi_encode_h264.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>>> index dd17be2..d6926c4 100644
>>> --- a/libavcodec/vaapi_encode_h264.c
>>> +++ b/libavcodec/vaapi_encode_h264.c
>>> @@ -350,7 +350,7 @@ static int
>>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>>       sps->chroma_format_idc    = 1;
>>>
>>>       sps->log2_max_frame_num_minus4 = 4;
>>> -    sps->pic_order_cnt_type        = 0;
>>> +    sps->pic_order_cnt_type        = ctx->max_b_depth ? 0 : 2;
>>
>>
>> pic_order_cnt_type (0) should work for ctx->max_b_depth == 0. If 2 is preferred
>> for your vaapi driver, it would be better to query the capability from the
>> driver, like as what commit 9f02e033875185409c861846f209b04a3be339d2 did.
> 
> It's not about an encoder, but rather about decoder. Some decoders
> (namely the Snapdragon HW decoder)
> will buffer frames when pic_order_cnt_type == 0 (in case the frames
> will be reordered?) which results
> in undesired increased latency. Setting pic_order_cnt_type to 2 will
> fix this problem, and it is also what
> libx264 does [0].

Has that decoder bug been reported to the vendor so that they can fix it?

The decoder should be reading the stream buffering from max_num_reorder_frames in the VUI parameters, which is set at as expected <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/vaapi_encode_h264.c;h=dd17be2190ab07331ed69cf830b8cea2584ad353;hb=HEAD#l478>.

The change to using POC type 2 is not unreasonable to save a few bits, but to do it you will also need to fix all of the POC values to actually match the type 2 behaviour (the current type 0 setup steps by 1 on frames because interlacing is not supported, but type 2 always steps by 2 - see §8.2.1.3).

- Mark
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] vaapi_encode_h264: Only set pic_order_cnt_type to 0 with B-frames

[David Rosca]

On Mon, Jan 9, 2023 at 10:11 PM Mark Thompson <sw@jkqxz.net> wrote:
>
> On 09/01/2023 07:37, David Rosca wrote:
> > On Mon, Jan 9, 2023 at 3:22 AM Xiang, Haihao <haihao.xiang@intel.com> wrote:
> >>
> >> On Do, 2022-12-29 at 22:20 +0100, David Rosca wrote:
> >>> ---
> >>>   libavcodec/vaapi_encode_h264.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> >>> index dd17be2..d6926c4 100644
> >>> --- a/libavcodec/vaapi_encode_h264.c
> >>> +++ b/libavcodec/vaapi_encode_h264.c
> >>> @@ -350,7 +350,7 @@ static int
> >>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
> >>>       sps->chroma_format_idc    = 1;
> >>>
> >>>       sps->log2_max_frame_num_minus4 = 4;
> >>> -    sps->pic_order_cnt_type        = 0;
> >>> +    sps->pic_order_cnt_type        = ctx->max_b_depth ? 0 : 2;
> >>
> >>
> >> pic_order_cnt_type (0) should work for ctx->max_b_depth == 0. If 2 is preferred
> >> for your vaapi driver, it would be better to query the capability from the
> >> driver, like as what commit 9f02e033875185409c861846f209b04a3be339d2 did.
> >
> > It's not about an encoder, but rather about decoder. Some decoders
> > (namely the Snapdragon HW decoder)
> > will buffer frames when pic_order_cnt_type == 0 (in case the frames
> > will be reordered?) which results
> > in undesired increased latency. Setting pic_order_cnt_type to 2 will
> > fix this problem, and it is also what
> > libx264 does [0].
>
> Has that decoder bug been reported to the vendor so that they can fix it?

I haven't and I'm not sure where I would check if it was already
reported by someone else. I wasn't even sure it's a bug since all
encoders I have tried so far (x264, nvenc, amf) will output
pic_order_cnt_type=2 when configured for low latency encoding.

>
> The decoder should be reading the stream buffering from max_num_reorder_frames in the VUI parameters, which is set at as expected <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/vaapi_encode_h264.c;h=dd17be2190ab07331ed69cf830b8cea2584ad353;hb=HEAD#l478>.
>
> The change to using POC type 2 is not unreasonable to save a few bits, but to do it you will also need to fix all of the POC values to actually match the type 2 behaviour (the current type 0 setup steps by 1 on frames because interlacing is not supported, but type 2 always steps by 2 - see §8.2.1.3).
>
> - Mark
> _______________________________________________
> 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".
_______________________________________________
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".