On 11/6/2023 8:37 AM, Anton Khirnov wrote:
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.

True. Will change.


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

Should i move the AVStream related functions to it too?
_______________________________________________
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".

Reply via email to