| Message ID | 20230412194936.48022-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2df4e054d4b8f69ce3c2c06aace9df9ba6d2ac2e |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avutil/frame: add new interlaced and top_field_first flags | expand |
| 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 |
On 4/12/2023 4:49 PM, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Missing version bump and APIChanges entry. > > libavutil/frame.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 5b58c14ac3..87e0a51226 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -586,6 +586,15 @@ typedef struct AVFrame { > * A flag to mark the frames which need to be decoded, but shouldn't be output. > */ > #define AV_FRAME_FLAG_DISCARD (1 << 2) > +/** > + * A flag to mark frames whose content is interlaced. > + */ > +#define AV_FRAME_FLAG_INTERLACED (1 << 3) > +/** > + * A flag to mark frames where the top field is displayed first if the content > + * is interlaced. > + */ > +#define AV_FRAME_FLAG_TOP_FIELD_FIRST (1 << 4) > /** > * @} > */ I'll be pushing this set soon unless there are objections.
Quoting James Almer (2023-04-12 21:49:32) > Signed-off-by: James Almer <jamrial@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.
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@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.
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@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.
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@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.
On 4/12/2023 4:49 PM, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Missing version bump and APIChanges entry. > > libavutil/frame.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 5b58c14ac3..87e0a51226 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -586,6 +586,15 @@ typedef struct AVFrame { > * A flag to mark the frames which need to be decoded, but shouldn't be output. > */ > #define AV_FRAME_FLAG_DISCARD (1 << 2) > +/** > + * A flag to mark frames whose content is interlaced. > + */ > +#define AV_FRAME_FLAG_INTERLACED (1 << 3) > +/** > + * A flag to mark frames where the top field is displayed first if the content > + * is interlaced. > + */ > +#define AV_FRAME_FLAG_TOP_FIELD_FIRST (1 << 4) > /** > * @} > */ If no one objects, I'll push this set and the key_frame one this week.
On 4/29/2023 9:19 PM, James Almer wrote: > On 4/12/2023 4:49 PM, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> Missing version bump and APIChanges entry. >> >> libavutil/frame.h | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/libavutil/frame.h b/libavutil/frame.h >> index 5b58c14ac3..87e0a51226 100644 >> --- a/libavutil/frame.h >> +++ b/libavutil/frame.h >> @@ -586,6 +586,15 @@ typedef struct AVFrame { >> * A flag to mark the frames which need to be decoded, but shouldn't >> be output. >> */ >> #define AV_FRAME_FLAG_DISCARD (1 << 2) >> +/** >> + * A flag to mark frames whose content is interlaced. >> + */ >> +#define AV_FRAME_FLAG_INTERLACED (1 << 3) >> +/** >> + * A flag to mark frames where the top field is displayed first if >> the content >> + * is interlaced. >> + */ >> +#define AV_FRAME_FLAG_TOP_FIELD_FIRST (1 << 4) >> /** >> * @} >> */ > > If no one objects, I'll push this set and the key_frame one this week. Applied.
diff --git a/libavutil/frame.h b/libavutil/frame.h index 5b58c14ac3..87e0a51226 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -586,6 +586,15 @@ typedef struct AVFrame { * A flag to mark the frames which need to be decoded, but shouldn't be output. */ #define AV_FRAME_FLAG_DISCARD (1 << 2) +/** + * A flag to mark frames whose content is interlaced. + */ +#define AV_FRAME_FLAG_INTERLACED (1 << 3) +/** + * A flag to mark frames where the top field is displayed first if the content + * is interlaced. + */ +#define AV_FRAME_FLAG_TOP_FIELD_FIRST (1 << 4) /** * @} */
Signed-off-by: James Almer <jamrial@gmail.com> --- Missing version bump and APIChanges entry. libavutil/frame.h | 9 +++++++++ 1 file changed, 9 insertions(+)