[FFmpeg-devel] [PATCH] libavutil/video_enc_params: add block type

[Lynne]

Jul 7, 2020, 19:22 by yonglel-at-google.com at ffmpeg.org:

> On Tue, Jul 7, 2020 at 12:59 AM Anton Khirnov <anton at khirnov.net> wrote:
>
>> Quoting Yongle Lin (2020-07-06 23:08:17)
>> > add block type field to AVVideoBlockParams so we could either export or
>> visualize it later.
>> > ---
>> >  libavutil/video_enc_params.h | 20 ++++++++++++++++++++
>> >  1 file changed, 20 insertions(+)
>>
>> We generally require new APIs to be immediately useful. So in this case,
>> there should also be a patch that makes some decoder export those
>> fields.
>>
>> >
>> > diff --git a/libavutil/video_enc_params.h b/libavutil/video_enc_params.h
>> > index 43fa443154..55b9fc4031 100644
>> > --- a/libavutil/video_enc_params.h
>> > +++ b/libavutil/video_enc_params.h
>> > @@ -101,6 +101,21 @@ typedef struct AVVideoEncParams {
>> >      int32_t delta_qp[4][2];
>> >  } AVVideoEncParams;
>> >
>> > +typedef struct MacroBlockType {
>> > +    /**
>> > +     * Is intra prediction
>> > +     */
>> > +    int intra;
>> > +    /**
>> > +     * Skip flag
>> > +     */
>> > +    int skip;
>>
>> These structures are stored per-block, so it seems pretty wasteful to
>> spend a whole int (4 bytes) when only one bit is used.
>>
> You are right. I should add bit fields to restrict the size. Do you think
> it makes more sense to unwrap the fields and put them all in
> AVVideoBlockParams or pack them in a struct like this? Thanks.
>

Put them in the AVVideoBlockParams struct. It was designed to be extended in that
way and currently has only 5 entries.
I'd suggest making it a uint64_t bitfield for all booleans that we might have to use
in the future.

As for the references field, I'd suggest making that at least 8 entries. We don't
know what the future would look like so even if its somewhat wasteful now,
we'll save ourselves a lot of trouble later if blocks may have more than 2 references.