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

[Mark Thompson]

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.
>>> + at 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