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 4BFA1475F5 for ; Thu, 14 Sep 2023 15:43:48 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A5F3168C709; Thu, 14 Sep 2023 18:43:45 +0300 (EEST) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id DBC8268C6F8 for ; Thu, 14 Sep 2023 18:43:39 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 945122401E7 for ; Thu, 14 Sep 2023 17:43:39 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavis, port 10024) with ESMTP id pfmcooVL0mmz for ; Thu, 14 Sep 2023 17:43:39 +0200 (CEST) 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 EC1832401A4 for ; Thu, 14 Sep 2023 17:43:38 +0200 (CEST) Received: by lain.khirnov.net (Postfix, from userid 1000) id C9B7D1601B9; Thu, 14 Sep 2023 17:43:38 +0200 (CEST) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: <20230906174431.45558-1-jamrial@gmail.com> References: <20230906174431.45558-1-jamrial@gmail.com> Mail-Followup-To: FFmpeg development discussions and patches Date: Thu, 14 Sep 2023 17:43:38 +0200 Message-ID: <169470621879.20400.7203675987362320361@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data 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: Quoting James Almer (2023-09-06 19:44:20) > Changes since the previous version: > - Removed the AVPacketSideDataSet from AVCodecContext, using instead the > existing coded_side_data field, at Anton's request. > > I still prefer using a new field of type AVPacketSideDataSet, given that the > user can currently only fill coded_side_data manually (See the implementation > in the codecpar copy functions, which non lavf users would need to replicate), > and more so considering coded_side_data is a field pretty much nothing but lavf > uses. > > Opinions are very welcome and needed. To provide more context on the issue: AVPackets can have side data attached to them, in the form of AVPacket.{side_data,side_data_elems} array+count. Some of this side data can occasionally be stored in global headers, e.g. in containers or extradata. We have some limited support for this: * AVStream.{side_data,nb_side_data} array+count allows demuxers to export this to user, and muxers to accept it from the user * AVCodecContext.{coded_side_data,nb_coded_side_data} allows decoders to accept it from the user (in principle, in practice it is not used), and encoders to export it to the user (this is used, but not very much) The broad idea for this set is adding stream-global "coded/packet" side data to AVCodecParameters, so that it can be naturally communicated from demuxers to bitstream filters to decoders, and from encoders to bitstream filters to muxers. Since AVStream already contains an AVCodecParameters instance, the abovementioned AVStream.side_data becomes redundant and is deprecated, the muxer/demuxer stream-global side data would be passed through AVStream.codecpar. The original version proposed by James adds a new struct, that bundles the side data array+count, and a set of helpers operating on this struct. Then this new struct is added to AVCodecParameters and AVCodecContext, which replaces the abovementioned AVStream and AVCodecContext side data array+count. Notably AVPacket is left as-is, because its side data is widely used and replacing array+count by this new struct would be a very intrusive break for little gain. My objections to this approach are * coded side data is now stored differently in AVPacket and in everything else * the case for replacing AVCodecContext.coded_side_data does not seem sufficiently strong to me My alternative suggestion was: * avoid adding a new struct (which merely bundles array+count) * add a set of helpers that accept array+count as parameters and operate on them * use these helpers for all operations on side data across AVPacket, AVCodecContext, and AVCodecParameters I have a moderately strong preference for this approach, but James disagrees. More opinions are very much welcome. -- 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".