Re: [FFmpeg-devel] [PATCH] [RFC]avformat: introduce AVStreamGroup

[Anton Khirnov <anton@khirnov.net>]

Quoting James Almer (2023-09-06 16:38:32)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> This is an initial proof of concept for AVStream groups, something that's
> needed for quite a few existing and upcoming formats that lavf has no way to
> currently export. Said formats define a single video or audio stream composed
> by merging several individualy multiplexed streams within a media file.
> This is the case of HEIF, a format defining a tiled image where each tile is a
> separate image (either hevc, av1, etc) all of which need to be decoded
> individualy and then stitched together for presentation using container level
> information; IAMF, a new audio format where several individual streams (mono or
> stereo) need to be decoded individually and then combined to form audio streams
> with different channel layouts; and MPEG-TS programs, currently exported as
> AVProgram, which this new general purpose API would replace.
> There may be others too, like something ISOBMFF specific and not HEIF related,
> I'm told.
> 
> A new struct, AVStreamGroup, would cover all these cases by grouping the
> relevant streams and propagating format specific metadata for the purpose of
> combining them into what's expected for presentation (Something a filter for
> example would have to take care of).
> 
> Missing from this first version is something like a type field, which could be
> an enum listing the different currently known formats the user would then use
> to interpret the attached metadata, defined perhaps in codecpar.extradata
> 
> I'd like to hear opinions and suggestions to improve and properly handle this.
> 
>  libavformat/avformat.c |  26 ++++++++
>  libavformat/avformat.h | 143 ++++++++++++++++++++++++++++++++++++++++-
>  libavformat/dump.c     |  65 +++++++++++++++++--
>  libavformat/internal.h |  28 ++++++++
>  libavformat/options.c  |  77 ++++++++++++++++++++++
>  5 files changed, 332 insertions(+), 7 deletions(-)
> 

> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 1916aa2dc5..d18eafb933 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1007,6 +1007,59 @@ typedef struct AVStream {
>      int pts_wrap_bits;
>  } AVStream;
>  
> +typedef struct AVStreamGroup {
> +    /**
> +     * A class for @ref avoptions. Set on stream creation.
                                             ^^^^^^
                                             group

> +     */
> +    const AVClass *av_class;
> +
> +    /**
> +     * Group index in AVFormatContext.
> +     */
> +    int index;

unsigned?

> +
> +    /**
> +     * Format-specific group ID.
> +     * decoding: set by libavformat
> +     * encoding: set by the user, replaced by libavformat if left unset
> +     */
> +    int id;

might want to make this 64bit

> +
> +    /**
> +     * Codec parameters associated with this stream group. Allocated and freed
> +     * by libavformat in avformat_new_stream_group() and avformat_free_context()
> +     * respectively.
> +     *
> +     * - demuxing: filled by libavformat on stream group creation or in
> +     *             avformat_find_stream_info()
> +     * - muxing: filled by the caller before avformat_write_header()
> +     */
> +    AVCodecParameters *codecpar;
> +
> +    void *priv_data;

Do we really need this?

> +
> +    /**
> +     * Number of elements in AVStreamGroup.stream_index.
> +     *
> +     * Set by av_stream_group_add_stream() and av_stream_group_new_stream(), must not
> +     * be modified by any other code.
> +     */
> +    int nb_stream_indexes;
> +
> +    /**
> +     * A list of indexes of streams in the group. New entries are created with
> +     * av_stream_group_add_stream() and av_stream_group_new_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().
> +     * - muxing: entries are created by the user before avformat_write_header().
> +     *
> +     * Freed by libavformat in avformat_free_context().
> +     */
> +    int *stream_index;

unsigned for both?

> @@ -1844,6 +1940,51 @@ const AVClass *av_stream_get_class(void);
>   */
>  AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
>  
> +/**
> + * Add a new stream to a stream group.
> + *
> + * When demuxing, it may be called by the demuxer in read_header(). If the
> + * flag AVFMTCTX_NOHEADER is set in s.ctx_flags, then it may also
> + * be called in read_packet().
> + *
> + * When muxing, may be called by the user before avformat_write_header() after
> + * having allocated a new group with avformat_new_stream_group().
> + *
> + * User is required to call avformat_free_context() to clean up the allocation
> + * by av_stream_group_new_stream().
> + *
> + * This is functionally the same as avformat_new_stream() while also adding the
> + * newly allocated stream to the group belonging to the media file.
> + *
> + * @param stg stream group belonging to a media file.
> + *
> + * @return newly created stream or NULL on error.
> + * @see av_stream_group_add_stream, avformat_new_stream_group.
> + */
> +AVStream *av_stream_group_new_stream(AVStreamGroup *stg);

Is there a big enough advantage to having this as a separate function?

> +
> +/**
> + * Add an already allocated stream to a stream group.
> + *
> + * When demuxing, it may be called by the demuxer in read_header(). If the
> + * flag AVFMTCTX_NOHEADER is set in s.ctx_flags, then it may also
> + * be called in read_packet().
> + *
> + * When muxing, may be called by the user before avformat_write_header() after
> + * having allocated a new group with avformat_new_stream_group() and stream with
> + * avformat_new_stream().
> + *
> + * User is required to call avformat_free_context() to clean up the allocation
> + * by av_stream_group_add_stream().
> + *
> + * @param stg stream group belonging to a media file.
> + * @param st  stream in the media file to add to the group.
> + *
> + * @return 0 on success, or a negative AVERROR otherwise.
> + * @see avformat_new_stream, av_stream_group_new_stream, avformat_new_stream_group.
> + */
> +int av_stream_group_add_stream(AVStreamGroup *stg, const AVStream *st);

It'd be nice to have the streamgroup-related functions consistenly
namespaced.

E.g.
* avformat_stream_group_add()
* avformat_stream_group_add_stream()
* ff_stream_group_free()
etc.

alternatively for the first two:
* avformat_stream_group_create()
* avformat_stream_group_extend()

-- 
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-request@ffmpeg.org with subject "unsubscribe".