diff mbox series

[FFmpeg-devel,v2] lavc/vaapi_encode: Add filler_data option

Message ID 20230807072801.14703-2-nowrep@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2] lavc/vaapi_encode: Add filler_data option | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

David Rosca Aug. 7, 2023, 7:27 a.m. UTC 
v2: Add description in encoders.texi
---
 doc/encoders.texi         | 3 +++
 libavcodec/vaapi_encode.c | 1 +
 libavcodec/vaapi_encode.h | 9 ++++++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

Comments

Xiang, Haihao Aug. 15, 2023, 8:02 a.m. UTC | #1 
On Ma, 2023-08-07 at 09:27 +0200, David Rosca wrote:
> v2: Add description in encoders.texi
> ---
>  doc/encoders.texi         | 3 +++
>  libavcodec/vaapi_encode.c | 1 +
>  libavcodec/vaapi_encode.h | 9 ++++++++-
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index 25d6b7f09e..f146942aa5 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -3963,6 +3963,9 @@ Set the allowed max size in bytes for each frame. If the
> frame size exceeds
>  the limitation, encoder will adjust the QP value to control the frame size.
>  Invalid in CQP rate control mode.
>  
> +@item filler_data
> +Insert filler data in CBR rate control mode to ensure target bitrate.
> +
>  @item rc_mode
>  Set the rate control mode to use.  A given driver may only support a subset
> of
>  modes.
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index bfca315a7a..f161c76304 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1860,6 +1860,7 @@ rc_mode_found:
>  #if VA_CHECK_VERSION(1, 3, 0)
>              .quality_factor     = rc_quality,
>  #endif
> +            .rc_flags.bits.disable_bit_stuffing = !ctx->filler_data,
>          };
>          vaapi_encode_add_global_param(avctx,
>                                        VAEncMiscParameterTypeRateControl,
> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> index a1e639f56b..a2170cb8b0 100644
> --- a/libavcodec/vaapi_encode.h
> +++ b/libavcodec/vaapi_encode.h
> @@ -198,6 +198,9 @@ typedef struct VAAPIEncodeContext {
>      // Max Frame Size
>      int             max_frame_size;
>  
> +    // Filler Data
> +    int             filler_data;
> +
>      // Explicitly set RC mode (otherwise attempt to pick from
>      // available modes).
>      int             explicit_rc_mode;
> @@ -490,7 +493,11 @@ int ff_vaapi_encode_close(AVCodecContext *avctx);
>      { "max_frame_size", \
>        "Maximum frame size (in bytes)",\
>        OFFSET(common.max_frame_size), AV_OPT_TYPE_INT, \
> -      { .i64 = 0 }, 0, INT_MAX, FLAGS }
> +      { .i64 = 0 }, 0, INT_MAX, FLAGS }, \
> +    { "filler_data", \
> +      "Enable filler data", \
> +      OFFSET(common.filler_data), AV_OPT_TYPE_BOOL, \
> +      { .i64 = 1 }, 0, 1, FLAGS }
>  
>  #define VAAPI_ENCODE_RC_MODE(name, desc) \
>      { #name, desc, 0, AV_OPT_TYPE_CONST, { .i64 = RC_MODE_ ## name }, \

Will apply

- Haihao
Mark Thompson Aug. 15, 2023, 7:09 p.m. UTC | #2 
On 15/08/2023 09:02, Xiang, Haihao wrote:
> On Ma, 2023-08-07 at 09:27 +0200, David Rosca wrote:
>> v2: Add description in encoders.texi
>> ---
>>   doc/encoders.texi         | 3 +++
>>   libavcodec/vaapi_encode.c | 1 +
>>   libavcodec/vaapi_encode.h | 9 ++++++++-
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>> index 25d6b7f09e..f146942aa5 100644
>> --- a/doc/encoders.texi
>> +++ b/doc/encoders.texi
>> @@ -3963,6 +3963,9 @@ Set the allowed max size in bytes for each frame. If the
>> frame size exceeds
>>   the limitation, encoder will adjust the QP value to control the frame size.
>>   Invalid in CQP rate control mode.
>>   
>> +@item filler_data
>> +Insert filler data in CBR rate control mode to ensure target bitrate.
>> +
>>   @item rc_mode
>>   Set the rate control mode to use.  A given driver may only support a subset
>> of
>>   modes.
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index bfca315a7a..f161c76304 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -1860,6 +1860,7 @@ rc_mode_found:
>>   #if VA_CHECK_VERSION(1, 3, 0)
>>               .quality_factor     = rc_quality,
>>   #endif
>> +            .rc_flags.bits.disable_bit_stuffing = !ctx->filler_data,
>>           };
>>           vaapi_encode_add_global_param(avctx,
>>                                         VAEncMiscParameterTypeRateControl,
>> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
>> index a1e639f56b..a2170cb8b0 100644
>> --- a/libavcodec/vaapi_encode.h
>> +++ b/libavcodec/vaapi_encode.h
>> @@ -198,6 +198,9 @@ typedef struct VAAPIEncodeContext {
>>       // Max Frame Size
>>       int             max_frame_size;
>>   
>> +    // Filler Data
>> +    int             filler_data;
>> +
>>       // Explicitly set RC mode (otherwise attempt to pick from
>>       // available modes).
>>       int             explicit_rc_mode;
>> @@ -490,7 +493,11 @@ int ff_vaapi_encode_close(AVCodecContext *avctx);
>>       { "max_frame_size", \
>>         "Maximum frame size (in bytes)",\
>>         OFFSET(common.max_frame_size), AV_OPT_TYPE_INT, \
>> -      { .i64 = 0 }, 0, INT_MAX, FLAGS }
>> +      { .i64 = 0 }, 0, INT_MAX, FLAGS }, \
>> +    { "filler_data", \
>> +      "Enable filler data", \
>> +      OFFSET(common.filler_data), AV_OPT_TYPE_BOOL, \
>> +      { .i64 = 1 }, 0, 1, FLAGS }
>>   
>>   #define VAAPI_ENCODE_RC_MODE(name, desc) \
>>       { #name, desc, 0, AV_OPT_TYPE_CONST, { .i64 = RC_MODE_ ## name }, \
> 
> Will apply

Where does this actually work?  Neither of the Intel drivers look at this field - seems like it might be a good idea to note that it is driver dependent and may do nothing, at least.

Thanks,

- Mark
Mark Thompson Aug. 15, 2023, 7:25 p.m. UTC | #3 
On 15/08/2023 20:09, Mark Thompson wrote:
> On 15/08/2023 09:02, Xiang, Haihao wrote:
>> On Ma, 2023-08-07 at 09:27 +0200, David Rosca wrote:
>>> v2: Add description in encoders.texi
>>> ---
>>>   doc/encoders.texi         | 3 +++
>>>   libavcodec/vaapi_encode.c | 1 +
>>>   libavcodec/vaapi_encode.h | 9 ++++++++-
>>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>>> index 25d6b7f09e..f146942aa5 100644
>>> --- a/doc/encoders.texi
>>> +++ b/doc/encoders.texi
>>> @@ -3963,6 +3963,9 @@ Set the allowed max size in bytes for each frame. If the
>>> frame size exceeds
>>>   the limitation, encoder will adjust the QP value to control the frame size.
>>>   Invalid in CQP rate control mode.
>>> +@item filler_data
>>> +Insert filler data in CBR rate control mode to ensure target bitrate.
>>> +
>>>   @item rc_mode
>>>   Set the rate control mode to use.  A given driver may only support a subset
>>> of
>>>   modes.
>>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>>> index bfca315a7a..f161c76304 100644
>>> --- a/libavcodec/vaapi_encode.c
>>> +++ b/libavcodec/vaapi_encode.c
>>> @@ -1860,6 +1860,7 @@ rc_mode_found:
>>>   #if VA_CHECK_VERSION(1, 3, 0)
>>>               .quality_factor     = rc_quality,
>>>   #endif
>>> +            .rc_flags.bits.disable_bit_stuffing = !ctx->filler_data,
>>>           };
>>>           vaapi_encode_add_global_param(avctx,
>>>                                         VAEncMiscParameterTypeRateControl,
>>> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
>>> index a1e639f56b..a2170cb8b0 100644
>>> --- a/libavcodec/vaapi_encode.h
>>> +++ b/libavcodec/vaapi_encode.h
>>> @@ -198,6 +198,9 @@ typedef struct VAAPIEncodeContext {
>>>       // Max Frame Size
>>>       int             max_frame_size;
>>> +    // Filler Data
>>> +    int             filler_data;
>>> +
>>>       // Explicitly set RC mode (otherwise attempt to pick from
>>>       // available modes).
>>>       int             explicit_rc_mode;
>>> @@ -490,7 +493,11 @@ int ff_vaapi_encode_close(AVCodecContext *avctx);
>>>       { "max_frame_size", \
>>>         "Maximum frame size (in bytes)",\
>>>         OFFSET(common.max_frame_size), AV_OPT_TYPE_INT, \
>>> -      { .i64 = 0 }, 0, INT_MAX, FLAGS }
>>> +      { .i64 = 0 }, 0, INT_MAX, FLAGS }, \
>>> +    { "filler_data", \
>>> +      "Enable filler data", \
>>> +      OFFSET(common.filler_data), AV_OPT_TYPE_BOOL, \
>>> +      { .i64 = 1 }, 0, 1, FLAGS }
>>>   #define VAAPI_ENCODE_RC_MODE(name, desc) \
>>>       { #name, desc, 0, AV_OPT_TYPE_CONST, { .i64 = RC_MODE_ ## name }, \
>>
>> Will apply
> 
> Where does this actually work?  Neither of the Intel drivers look at this field - seems like it might be a good idea to note that it is driver dependent and may do nothing, at least.

Right, it got implemented in Mesa not that long ago.  So the field got turned into "pad all CBR streams with filler unless you set this bit which wasn't used before so is obviously zero" - ouch.

Do you have a use for the option, or are you just trying to fix the default?

If you really do want the option, I'm not sure it makes sense as a generic - it only makes sense for some codecs.  It would be helpful to say how the padding works, too (NAL filler, SEI filler, trailing zeroes?).

We should probably patch all of the release branches with "disable_bit_stuffing = 1" so they don't pad unexpectedly with new Mesa versions, too.

Thanks,

- Mark
David Rosca Aug. 15, 2023, 8:07 p.m. UTC | #4 
On Tue, Aug 15, 2023 at 9:25 PM Mark Thompson <sw@jkqxz.net> wrote:
>
> On 15/08/2023 20:09, Mark Thompson wrote:
> > On 15/08/2023 09:02, Xiang, Haihao wrote:
> >> On Ma, 2023-08-07 at 09:27 +0200, David Rosca wrote:
> >>> v2: Add description in encoders.texi
> >>> ---
> >>>   doc/encoders.texi         | 3 +++
> >>>   libavcodec/vaapi_encode.c | 1 +
> >>>   libavcodec/vaapi_encode.h | 9 ++++++++-
> >>>   3 files changed, 12 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/doc/encoders.texi b/doc/encoders.texi
> >>> index 25d6b7f09e..f146942aa5 100644
> >>> --- a/doc/encoders.texi
> >>> +++ b/doc/encoders.texi
> >>> @@ -3963,6 +3963,9 @@ Set the allowed max size in bytes for each frame. If the
> >>> frame size exceeds
> >>>   the limitation, encoder will adjust the QP value to control the frame size.
> >>>   Invalid in CQP rate control mode.
> >>> +@item filler_data
> >>> +Insert filler data in CBR rate control mode to ensure target bitrate.
> >>> +
> >>>   @item rc_mode
> >>>   Set the rate control mode to use.  A given driver may only support a subset
> >>> of
> >>>   modes.
> >>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> >>> index bfca315a7a..f161c76304 100644
> >>> --- a/libavcodec/vaapi_encode.c
> >>> +++ b/libavcodec/vaapi_encode.c
> >>> @@ -1860,6 +1860,7 @@ rc_mode_found:
> >>>   #if VA_CHECK_VERSION(1, 3, 0)
> >>>               .quality_factor     = rc_quality,
> >>>   #endif
> >>> +            .rc_flags.bits.disable_bit_stuffing = !ctx->filler_data,
> >>>           };
> >>>           vaapi_encode_add_global_param(avctx,
> >>>                                         VAEncMiscParameterTypeRateControl,
> >>> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> >>> index a1e639f56b..a2170cb8b0 100644
> >>> --- a/libavcodec/vaapi_encode.h
> >>> +++ b/libavcodec/vaapi_encode.h
> >>> @@ -198,6 +198,9 @@ typedef struct VAAPIEncodeContext {
> >>>       // Max Frame Size
> >>>       int             max_frame_size;
> >>> +    // Filler Data
> >>> +    int             filler_data;
> >>> +
> >>>       // Explicitly set RC mode (otherwise attempt to pick from
> >>>       // available modes).
> >>>       int             explicit_rc_mode;
> >>> @@ -490,7 +493,11 @@ int ff_vaapi_encode_close(AVCodecContext *avctx);
> >>>       { "max_frame_size", \
> >>>         "Maximum frame size (in bytes)",\
> >>>         OFFSET(common.max_frame_size), AV_OPT_TYPE_INT, \
> >>> -      { .i64 = 0 }, 0, INT_MAX, FLAGS }
> >>> +      { .i64 = 0 }, 0, INT_MAX, FLAGS }, \
> >>> +    { "filler_data", \
> >>> +      "Enable filler data", \
> >>> +      OFFSET(common.filler_data), AV_OPT_TYPE_BOOL, \
> >>> +      { .i64 = 1 }, 0, 1, FLAGS }
> >>>   #define VAAPI_ENCODE_RC_MODE(name, desc) \
> >>>       { #name, desc, 0, AV_OPT_TYPE_CONST, { .i64 = RC_MODE_ ## name }, \
> >>
> >> Will apply
> >
> > Where does this actually work?  Neither of the Intel drivers look at this field - seems like it might be a good idea to note that it is driver dependent and may do nothing, at least.
>
> Right, it got implemented in Mesa not that long ago.  So the field got turned into "pad all CBR streams with filler unless you set this bit which wasn't used before so is obviously zero" - ouch.
>
> Do you have a use for the option, or are you just trying to fix the default?
>
> If you really do want the option, I'm not sure it makes sense as a generic - it only makes sense for some codecs.  It would be helpful to say how the padding works, too (NAL filler, SEI filler, trailing zeroes?).
>
> We should probably patch all of the release branches with "disable_bit_stuffing = 1" so they don't pad unexpectedly with new Mesa versions, too.

The filler data was always enabled in Mesa, only recently it's
possible to disable it with disable_bit_stuffing = 1. So the use of
this option would be to disable this behavior.

AMD encoder uses NAL filler for H264/H265.

David

>
> Thanks,
>
> - 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".
diff mbox series

Patch

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 25d6b7f09e..f146942aa5 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -3963,6 +3963,9 @@  Set the allowed max size in bytes for each frame. If the frame size exceeds
 the limitation, encoder will adjust the QP value to control the frame size.
 Invalid in CQP rate control mode.
 
+@item filler_data
+Insert filler data in CBR rate control mode to ensure target bitrate.
+
 @item rc_mode
 Set the rate control mode to use.  A given driver may only support a subset of
 modes.
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index bfca315a7a..f161c76304 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1860,6 +1860,7 @@  rc_mode_found:
 #if VA_CHECK_VERSION(1, 3, 0)
             .quality_factor     = rc_quality,
 #endif
+            .rc_flags.bits.disable_bit_stuffing = !ctx->filler_data,
         };
         vaapi_encode_add_global_param(avctx,
                                       VAEncMiscParameterTypeRateControl,
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index a1e639f56b..a2170cb8b0 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -198,6 +198,9 @@  typedef struct VAAPIEncodeContext {
     // Max Frame Size
     int             max_frame_size;
 
+    // Filler Data
+    int             filler_data;
+
     // Explicitly set RC mode (otherwise attempt to pick from
     // available modes).
     int             explicit_rc_mode;
@@ -490,7 +493,11 @@  int ff_vaapi_encode_close(AVCodecContext *avctx);
     { "max_frame_size", \
       "Maximum frame size (in bytes)",\
       OFFSET(common.max_frame_size), AV_OPT_TYPE_INT, \
-      { .i64 = 0 }, 0, INT_MAX, FLAGS }
+      { .i64 = 0 }, 0, INT_MAX, FLAGS }, \
+    { "filler_data", \
+      "Enable filler data", \
+      OFFSET(common.filler_data), AV_OPT_TYPE_BOOL, \
+      { .i64 = 1 }, 0, 1, FLAGS }
 
 #define VAAPI_ENCODE_RC_MODE(name, desc) \
     { #name, desc, 0, AV_OPT_TYPE_CONST, { .i64 = RC_MODE_ ## name }, \