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".
