Hi, Quoting James Almer (2023-10-30 16:23:52) > Signed-off-by: James Almer <jamr...@gmail.com> > --- > No changes since last version. I'm resending this for the IAMF demuxer. > > I need opinions or reviews for this. We need to get this right from the start > and i don't want to push something that will afterwards be considered > unoptimal
The API generally looks good to me. > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index 9e7eca007e..9b2ee7ff14 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -1018,6 +1018,77 @@ typedef struct AVStream { > int pts_wrap_bits; > } AVStream; > > +enum AVStreamGroupParamsType { > + AV_STREAM_GROUP_PARAMS_NONE, > +}; > + > +typedef struct AVStreamGroup { > + /** > + * A class for @ref avoptions. Set on group creation. > + */ > + const AVClass *av_class; > + > + void *priv_data; > + > + /** > + * Group index in AVFormatContext. > + */ > + unsigned int index; > + > + /** > + * Group-specific group ID. type-specific? > + * > + * decoding: set by libavformat > + * encoding: set by the user, replaced by libavformat if left unset 'may be set' is the terminology we use for optional values > + */ > + int64_t id; > + > + /** > + * Group-specific type drop '-specific', I don't see what it adds to the meaning. > + * > + * decoding: set by libavformat on group creation > + * encoding: set by the user on group creation Could just say 'set by avformat_stream_group_create()', the user should never set it manually. > + */ > + enum AVStreamGroupParamsType type; > + > + /** > + * Group-specific type parameters type-specific? > + */ > + union { > + uintptr_t dummy; // Placeholder > + } params; > + > + /** > + * Metadata that applies to the whole file. s/file/group > + * > + * - demuxing: set by libavformat in avformat_open_input() Set on group creation, that does not have to be in avformat_open_input(). > + * - muxing: may be set by the caller before avformat_write_header() > + * > + * Freed by libavformat in avformat_free_context(). > + */ > + AVDictionary *metadata; > + > + /** > + * Number of elements in AVStreamGroup.streams. > + * > + * Set by avformat_stream_group_add_stream() must not be modified by any > other code. > + */ > + unsigned int nb_streams; > + > + /** > + * A list of streams in the group. New entries are created with > + * avformat_stream_group_add_stream(). > + * > + * - demuxing: entries are created by libavformat in > avformat_open_input(). > + * If AVFMTCTX_NOHEADER is set in ctx_flags, then new > entries may also > + * appear in av_read_frame(). We might want to make this type-specific, i.e. some group types are guaranteed to never change. > + * - muxing: entries are created by the user before > avformat_write_header(). > + * > + * Freed by libavformat in avformat_free_context(). > + */ > + const AVStream **streams; Is this const useful? I imagine some code might reasonably want to modify the stream through this pointer. > @@ -608,6 +629,18 @@ void ff_free_stream(AVStream **st); > */ > void ff_remove_stream(AVFormatContext *s, AVStream *st); > > +/** > + * Frees a stream group without modifying the corresponding AVFormatContext. > + * Must only be called if the latter doesn't matter or if the stream > + * is not yet attached to an AVFormatContext. > + */ > +void ff_free_stream_group(AVStreamGroup **pstg); ff_stream_group_free() Also, I'd prefer a dedicated header for this, internal.h is an abomination. -- Anton Khirnov _______________________________________________ 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".