On 11/21/2023 6:57 PM, Marton Balint wrote:


On Tue, 21 Nov 2023, James Almer wrote:

Signed-off-by: James Almer <jamr...@gmail.com>
---
libavformat/avformat.c | 178 +++++++++++++++++++++++++++++++++++++++--
libavformat/avformat.h | 163 +++++++++++++++++++++++++++++++++++++
libavformat/dump.c     |  33 ++++++--
libavformat/internal.h |  33 ++++++++
libavformat/options.c  |  90 +++++++++++++++++++++
5 files changed, 486 insertions(+), 11 deletions(-)

diff --git a/libavformat/avformat.c b/libavformat/avformat.c
index 5b8bb7879e..4e31924c71 100644
--- a/libavformat/avformat.c
+++ b/libavformat/avformat.c

[...]

@@ -493,6 +524,46 @@ static int match_stream_specifier(const AVFormatContext *s, const AVStream *st,
                match = 0;
            if (nopic && (st->disposition & AV_DISPOSITION_ATTACHED_PIC))
                match = 0;
+        } else if (*spec == 'g' && *(spec + 1) == ':') {
+            int64_t group_idx = -1, group_id = -1;
+            int found = 0;
+            char *endptr;
+            spec += 2;
+            if (*spec == '#' || (*spec == 'i' && *(spec + 1) == ':')) {
+                spec += 1 + (*spec == 'i');
+                group_id = strtol(spec, &endptr, 0);
+                if (spec == endptr || (*endptr && *endptr++ != ':'))
+                    return AVERROR(EINVAL);
+                spec = endptr;
+            } else {
+                group_idx = strtol(spec, &endptr, 0);
+                /* Disallow empty id and make sure that if we are not at the end, then another specifier must follow. */
+                if (spec == endptr || (*endptr && *endptr++ != ':'))
+                    return AVERROR(EINVAL);
+                spec = endptr;
+            }
+            if (match) {
+                if (group_id > 0) {
+                    for (unsigned i = 0; i < s->nb_stream_groups; i++) {
+                        if (group_id == s->stream_groups[i]->id) {
+                            group_idx = i;
+                            break;
+                        }
+                    }
+                }
+                if (group_idx < 0 || group_idx > s->nb_stream_groups)
+                    return AVERROR(EINVAL);
+                for (unsigned j = 0; j < s->stream_groups[group_idx]->nb_streams; j++) { +                    if (st->index == s->stream_groups[group_idx]->streams[j]->index) {
+                        found = 1;
+                        if (g)
+                            *g = s->stream_groups[group_idx];
+                        break;
+                    }
+                }
+            }
+            if (!found)
+                match = 0;

The documentation for the stream specifier changes are missing from
doc/fftools-common-opts.texi. You should update relevant docs preferably
in this patch...

Will do.


[...]

--- 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 by avformat_stream_group_create().
+     */
+    const AVClass *av_class;
+
+    void *priv_data;
+
+    /**
+     * Group index in AVFormatContext.
+     */
+    unsigned int index;
+
+    /**
+     * Group type-specific group ID.
+     *
+     * decoding: set by libavformat
+     * encoding: may set by the user, replaced by libavformat if left unset
+     */
+    int64_t id;

I don't quite get the encoding case where libavformat replaces the unset ID... Also, what is unset? 0 or -1? Because as far as I see 0 is the default, maybe -1 is better if unset has some special meaning?

I copied the doxy from the relevant AVStream field. It may be bogus there though, seeing for example how the mp4 muxer has an option to use the ids when muxing, but ultimately trusting them and not changing them.

Can remove that part here and just leave it as may be set by the user.


[...]

diff --git a/libavformat/internal.h b/libavformat/internal.h
index 7702986c9c..c6181683ef 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -202,6 +202,7 @@ typedef struct FFStream {
     */
    AVStream pub;

+    AVFormatContext *fmtctx;
    /**
     * Set to 1 if the codec allows reordering, so pts can be different
     * from dts.
@@ -427,6 +428,26 @@ static av_always_inline const FFStream *cffstream(const AVStream *st)
    return (const FFStream*)st;
}

+typedef struct FFStreamGroup {
+    /**
+     * The public context.
+     */
+    AVStreamGroup pub;
+
+    AVFormatContext *fmtctx;

If the only purpose of storing this is sanity checking if the user calls API with matching stream and stream group, then maybe better to make it ptrdiff_t, so nobody will try to misuse this to access the format context or do logging, etc...

It's the same in AVStream. And why couldn't a module access it for logging or similar? It's internal, not exposed, and the group is strictly a part of its parent AVFormatContext.


Regards,
Marton
_______________________________________________
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".
_______________________________________________
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