On Fri, Dec 10, 2021 at 12:06 AM Marton Balint <c...@passwd.hu> wrote:
>
>
>
> On Thu, 9 Dec 2021, Anton Khirnov wrote:
>
> > Quoting Lynne (2021-12-08 13:57:45)
> >> 8 Dec 2021, 13:16 by an...@khirnov.net:
> >>
> >>> Quoting Lynne (2021-12-08 10:02:34)
> >>>
> >>>> 8 Dec 2021, 02:06 by jamr...@gmail.com:
> >>>>
> >>>>> From: Anton Khirnov <an...@khirnov.net>
> >>>>>
> >>>>> The new API is more extensible and allows for custom layouts.
> >>>>> More accurate information is exported, eg for decoders that do not
> >>>>> set a channel layout, lavc will not make one up for them.
> >>>>>
> >>>>> Deprecate the old API working with just uint64_t bitmasks.
> >>>>>
> >>>>> Expanded and completed by Vittorio Giovara <vittorio.giov...@gmail.com>
> >>>>> and James Almer <jamr...@gmail.com>.
> >>>>> Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
> >>>>> Signed-off-by: James Almer <jamr...@gmail.com>
> >>>>> ---
> >>>>> libavutil/channel_layout.c | 498 ++++++++++++++++++++++++++++++------
> >>>>> libavutil/channel_layout.h | 510 ++++++++++++++++++++++++++++++++++---
> >>>>> libavutil/version.h | 1 +
> >>>>> 3 files changed, 906 insertions(+), 103 deletions(-)
> >>>>>
> >>>>> /**
> >>>>> * @}
> >>>>> @@ -128,6 +199,157 @@ enum AVMatrixEncoding {
> >>>>> AV_MATRIX_ENCODING_NB
> >>>>> };
> >>>>>
> >>>>> +/**
> >>>>> + * @}
> >>>>> + */
> >>>>> +
> >>>>> +/**
> >>>>> + * An AVChannelCustom defines a single channel within a custom order
> >>>>> layout
> >>>>> + *
> >>>>> + * Unlike most structures in FFmpeg, sizeof(AVChannelCustom) is a part
> >>>>> of the
> >>>>> + * public ABI.
> >>>>> + *
> >>>>> + * No new fields may be added to it without a major version bump.
> >>>>> + */
> >>>>> +typedef struct AVChannelCustom {
> >>>>> + enum AVChannel id;
> >>>>> +} AVChannelCustom;
> >>>>>
> >>>>
> >>>> Consider adding a 25-byte or so of a description field string here that
> >>>> would also be copied if the frame/layout is copied?
> >>>> That way, users can assign a description label to each channel,
> >>>> for example labeling each channel in a 255 channel custom layout
> >>>> Opus stream used in production at a venue somewhere.
> >>>> Also, consider additionally reserving 16-bytes here, just in case.
> >>>>
> >>>
> >>> That would enlarge the layout size by a significant amount. Keep in mind
> >>> that this is all per frame. And for something that very few people would
> >>> want to use.
> >>>
> >>> These descriptors, if anyone really needs them, can live in side data.
> >>>
> >>
> >> That's fine with me.
> >> Can we at least have an opaque uint64_t? I'd accept a uintptr_t,
> >> or an int too, or even a single-byte uint8_t.
> >
> > like this?
> >
> > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
> > index 7b77a74b61..7b2ba12532 100644
> > --- a/libavutil/channel_layout.h
> > +++ b/libavutil/channel_layout.h
> > @@ -252,7 +252,8 @@ enum AVMatrixEncoding {
> > * No new fields may be added to it without a major version bump.
> > */
> > typedef struct AVChannelCustom {
> > - enum AVChannel id;
> > + enum AVChannel id:16;
> > + int reserved:16;
> > } AVChannelCustom;
>
> A dynamically allocated field is so bad here? E.g.
>
> AVDictionary *metadata
>
> You can store labels. You can store group names. You can store language.
> You can store sound positions, GPS coordiantes, whatever.
>
> Yes, it will be slow. But it is rarely used for in normal cases, and it
> can be NULL usually.
>
Pointers add a lot of trouble with ownership and memory management.
Wouldn't be so bad if its just the pointer without all that management
baggage.
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
