[FFmpeg-devel] [PATCH 1/5] avutil/frame: add new interlaced and top_field_first flags

[James Almer]

On 4/17/2023 8:51 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-04-17 13:32:16)
>> On 4/17/2023 7:49 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-04-12 21:49:32)
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>> Missing version bump and APIChanges entry.
>>>>
>>>>    libavutil/frame.h | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)
>>>>
>>>
>>> Changing all those into bitfields of size 1 might be better, because no
>>> code would need to be changed.
>>
>> Can you elaborate on this? If i do
>>
>>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>>> index f85d630c5c..3f3deab657 100644
>>> --- a/libavutil/frame.h
>>> +++ b/libavutil/frame.h
>>> @@ -494,12 +494,12 @@ typedef struct AVFrame {
>>>       /**
>>>        * The content of the picture is interlaced.
>>>        */
>>> -    int interlaced_frame;
>>> +    int interlaced_frame: 1;
>>>
>>>       /**
>>>        * If the content is interlaced, is top field displayed first.
>>>        */
>>> -    int top_field_first;
>>> +    int top_field_first: 1;
>>>
>>>       /**
>>>        * Tell user application that palette has changed from previous frame.
>>
>> It's not only an ABI break, but i assume the compiler will still
>> pad/align the struct for the next field, so you're not saving many bytes.
> 
> Yes it's an ABI break, so the savings will only appear at the next major
> bump. But with a deprecation we'd have to wait even longer.
> 
> The idea is to put all these fields next to each other so they'd form a
> single unit. We could also reduce flags to 8 or 16 bits until/unless we
> need more. Then all of these would fit in the size of a single int.

So, on IRC it was decided that bitfields is not the way to go as it's 
implementation dependent, plus it would break FFI usage of our headers. 
As such, this set and the key_frame one are still open.