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 53182475EF for ; Thu, 14 Sep 2023 16:40:31 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 99A1168C7E3; Thu, 14 Sep 2023 19:40:27 +0300 (EEST) Received: from mail-ot1-f53.google.com (mail-ot1-f53.google.com [209.85.210.53]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5CAA068C6E2 for ; Thu, 14 Sep 2023 19:40:21 +0300 (EEST) Received: by mail-ot1-f53.google.com with SMTP id 46e09a7af769-6c0b727c1caso680512a34.0 for ; Thu, 14 Sep 2023 09:40:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694709619; x=1695314419; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=vY4lHEhupvBgiKz7s+tfnDEyZAcsjzIUUzsL2b6o0s4=; b=nIdmfMmu3HYayGioQPHIurPk0fvyf5C/gB09tYKHrYFW+V/5j8o22ANlhFE5Ww+Zlx HResqd4zHH0E6l/VXtrbELHjNtDMwH4Hri2prPa/z+8ShDhuDoNBK+Xy1iSmgblXS+7e u5IY7RjE0oY3d3YYFSK0pWiY2H0+TIYcsm0XXd6iX70m8I/uhMdRSs4raX2GxKoLd7Bo BWKNVzjX/BwCmk++2qezf3fX8IO4jvI47bs+2ivzRt6OG700UQWuDo5HURpJDUR728UN EKdjrKQWDKNYYQfL2QRWQ3CxBi4wRYOD5xysattyAPBzH3c/rJ6sS3yJTmUDBTA7TtNh V3cw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694709619; x=1695314419; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vY4lHEhupvBgiKz7s+tfnDEyZAcsjzIUUzsL2b6o0s4=; b=A1liclrOxCq9HSVqpeOnwJYB2tj1ExY12jfhT2mLuqQlnBgzGUgw89vPRbzW+6Fdr/ Br6cu6ZynqMpTyq6Frsx7mKcQK6z9xDVWudRzaVNIaXzP3cLP3L6sYRZgEgbpPwVmpf/ BK626vo/YF1an6fQPh/QSiXmQNp0Y0pN+ufmOYAqLWo6X8vYVdbU62xGFkQIJ8qtPCbf ZjxHmYYzWgWTcfZBTozPDmwCrx9JI513B6u6XQkZ0eexyeJlbg7QB45qHCReODVzgHPC kv/U+eVQ/XmqPtefZrtwU06ayAA1y4+YwfLU+6oijTijgcxt2eS5TtE9T09LzP/5U952 Va3A== X-Gm-Message-State: AOJu0Yxh1kudtfZokH2fJGfpMWfwhBtXXQ/Apdr2xGamHM3eAUA5vm4G Gn6XPENmqW/XRXGV2neN5on3c+jBzjg= X-Google-Smtp-Source: AGHT+IEGqvOxgt4uuMCpp66OzMOTUQy4m4RGabrFcejB01giXTFfMzdgXv02KVAKiQkebN0uO5KsZw== X-Received: by 2002:a9d:6f04:0:b0:6b8:9a3a:ea12 with SMTP id n4-20020a9d6f04000000b006b89a3aea12mr6986436otq.12.1694709619162; Thu, 14 Sep 2023 09:40:19 -0700 (PDT) Received: from [192.168.0.10] (host197.190-225-105.telecom.net.ar. [190.225.105.197]) by smtp.gmail.com with ESMTPSA id x14-20020a056830114e00b006b8a2cd44adsm791892otq.72.2023.09.14.09.40.17 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Sep 2023 09:40:18 -0700 (PDT) Message-ID: Date: Thu, 14 Sep 2023 13:40:20 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 To: ffmpeg-devel@ffmpeg.org References: <20230906174431.45558-1-jamrial@gmail.com> <169470621879.20400.7203675987362320361@lain.khirnov.net> Content-Language: en-US From: James Almer In-Reply-To: <169470621879.20400.7203675987362320361@lain.khirnov.net> 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 9/14/2023 12:43 PM, Anton Khirnov wrote: > 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. In the future, packet side data could be made ref counted. For this, we'd need to add a new AVBufferRef field to AVPacketSideData, which is currently tied to the ABI. Ideally, if this happens, sizeof(AVPacketSideData) would be made not a part of the ABI, putting it in line with the AVFrame counterpart. Doing this is pretty much not an option for the existing array+count fields in AVPacket and AVCodecContext, because everyone, from external API users to our own API users like the CLI last i checked, loop through it manually as nothing prevents them to. But if a new field of the new set struct is added as replacement (a struct where the AVPacketSideData field is an array of pointers, and whose helpers can be defined as "must use to handle this struct"), then this would not be an issue. I probably said it several times, including in the above paragraph, but one of my objectives is trying to sync frame and packet side data structs, fields and helpers, since right now both are considerably different. Jan has a patchset also introducing a side data set for frames, so this is the time to try and align both as much as possible (which i already talked to him about) and laying the ground for future changes. > > 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 It's used only by a handful of encoders to export CPB side data, which probably only lavf looks at, and by no decoder at all. It's a field that would be completely painless to replace. > > 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 See above my comment about sizeof(AVPacketSideData) and users accessing the array it directly. > * 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. > _______________________________________________ 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".