Apr 17, 2023, 19:27 by [email protected]: > On 4/17/2023 2:25 PM, Lynne wrote: > >> Apr 17, 2023, 17:15 by [email protected]: >> >>> On 4/17/2023 12:00 PM, Lynne wrote: >>> >>>> Apr 17, 2023, 16:40 by [email protected]: >>>> >>>>> Should reduce the size of AVFrame in the next major bump without changing >>>>> the API. >>>>> >>>>> Suggested-by: Anton Khirnov <[email protected]> >>>>> Signed-off-by: James Almer <[email protected]> >>>>> --- >>>>> This supersedes "avutil/frame: add new interlaced and top_field_first >>>>> flags" >>>>> and "avutil/frame: add a keyframe flag to AVFrame". >>>>> >>>>> libavutil/frame.h | 56 +++++++++++++++++++++++++++++++++++++++++++++ >>>>> libavutil/version.h | 1 + >>>>> 2 files changed, 57 insertions(+) >>>>> >>>>> diff --git a/libavutil/frame.h b/libavutil/frame.h >>>>> index f85d630c5c..c26067f383 100644 >>>>> --- a/libavutil/frame.h >>>>> +++ b/libavutil/frame.h >>>>> @@ -416,6 +416,7 @@ typedef struct AVFrame { >>>>> */ >>>>> int format; >>>>> +#if FF_API_BITFIELDS >>>>> /** >>>>> * 1 -> keyframe, 0-> not >>>>> */ >>>>> @@ -425,6 +426,57 @@ typedef struct AVFrame { >>>>> * Picture type of the frame. >>>>> */ >>>>> enum AVPictureType pict_type; >>>>> +#else >>>>> + /** >>>>> + * 1 -> keyframe, 0-> not >>>>> + */ >>>>> + unsigned int key_frame: 1; >>>>> + >>>>> + /** >>>>> + * The content of the picture is interlaced. >>>>> + */ >>>>> + unsigned int interlaced_frame: 1; >>>>> + >>>>> + /** >>>>> + * If the content is interlaced, is top field displayed first. >>>>> + */ >>>>> + unsigned int top_field_first: 1; >>>>> + >>>>> + /** >>>>> + * Tell user application that palette has changed from previous >>>>> frame. >>>>> + */ >>>>> + unsigned int palette_has_changed: 1; >>>>> + >>>>> + /** >>>>> + * Reserved. Must not be touched. >>>>> + */ >>>>> + unsigned int reserved_bitfield: (sizeof(unsigned int) * 8) - 9; >>>>> + >>>>> + /** >>>>> + * MPEG vs JPEG YUV range. >>>>> + * - encoding: Set by user >>>>> + * - decoding: Set by libavcodec >>>>> + */ >>>>> + enum AVColorRange color_range: 2; >>>>> + >>>>> + enum AVChromaLocation chroma_location: 3; >>>>> >>>> >>>> Definitely disagree on all non-8bit field limits. >>>> The reserved_bitfield is especially ugly. >>>> A few wasted bits wouldn't affect much, we don't even support building on >>>> 6502s. >>>> Use bools, or limit them to 8bits so we can use bools when we bump? >>>> The rest can be limited to 8bits. >>>> >>> >>> I added reserved_bitfield to turn what otherwise will be compiler-injected >>> padding into something that can be reused if we were to add new fields here >>> instead of at the end of the struct. I can remove it if you prefer, and >>> make color_range and chroma_location into :8. >>> Like i told you on IRC, i want to keep these as enum and not change their >>> type to uint8_t, _Bool, or anything like that. Also, i wouldn't be >>> surprised if using _Bool breaks some old weird compilers. >>> >>> With this change, I'm replacing 40 bytes worth of fields with 8 bytes worth >>> of fields with no API break. >>> >> >> I'm fine with enums staying as enums, and limiting them to 8 bits. >> I'm not fine with limiting flags to 1 bit or 2/3 bits. I'd like for them to >> be limited to 8 bits, and changing their type to bool or uint8_t at the >> bump. You're still saving at least 21 bytes. >> > > But why use 8 bits when we can use 1? >
No need for explicit padding. They're able to be set via pointers, simplifying FFI. They're faster to access. We could need to extend color_range and chroma_location in the future. It's not worth saving a few tens of bits in a large struct that's going to have to grow larger if we want to support e.g. subtitles in avframes. _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
