From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 064A747F23 for ; Mon, 6 Nov 2023 11:37:34 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0760A68C851; Mon, 6 Nov 2023 13:37:32 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id ECEA068C944 for ; Mon, 6 Nov 2023 13:37:24 +0200 (EET) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id A8CF72405F2 for ; Mon, 6 Nov 2023 12:37:24 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavis, port 10024) with ESMTP id RgZcfhn3qlFB for ; Mon, 6 Nov 2023 12:37:24 +0100 (CET) Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id F34512405EF for ; Mon, 6 Nov 2023 12:37:23 +0100 (CET) Received: by lain.khirnov.net (Postfix, from userid 1000) id CAED71601B9; Mon, 6 Nov 2023 12:37:23 +0100 (CET) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: <20231030152354.2818-1-jamrial@gmail.com> References: <20231030152354.2818-1-jamrial@gmail.com> Mail-Followup-To: FFmpeg development discussions and patches Date: Mon, 06 Nov 2023 12:37:23 +0100 Message-ID: <169927064380.11195.2245547213925263654@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH v3 1/3] [PoC]avformat: introduce AVStreamGroup X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Hi, Quoting James Almer (2023-10-30 16:23:52) > Signed-off-by: James Almer > --- > 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-request@ffmpeg.org with subject "unsubscribe".