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 E2E274008E for ; Sun, 19 Dec 2021 11:35:25 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3C8B668A56A; Sun, 19 Dec 2021 13:35:22 +0200 (EET) Received: from iq.passwd.hu (iq.passwd.hu [217.27.212.140]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C655E6804B9 for ; Sun, 19 Dec 2021 13:35:15 +0200 (EET) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id 13685E649A for ; Sun, 19 Dec 2021 12:35:14 +0100 (CET) X-Virus-Scanned: amavisd-new at passwd.hu Received: from iq.passwd.hu ([127.0.0.1]) by localhost (iq.passwd.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id b7Mvn3kB5bgq for ; Sun, 19 Dec 2021 12:35:12 +0100 (CET) Received: from iq (iq [217.27.212.140]) by iq.passwd.hu (Postfix) with ESMTPS id 09186E6403 for ; Sun, 19 Dec 2021 12:35:12 +0100 (CET) Date: Sun, 19 Dec 2021 12:35:11 +0100 (CET) From: Marton Balint To: FFmpeg development discussions and patches In-Reply-To: <20211218141516.GM2829255@pb2> Message-ID: <31397d1-438-8bfd-5a79-a07a67982559@passwd.hu> References: <20211216132151.8216-1-jamrial@gmail.com> <92861361-f316-78a5-3c72-1e279d2a9f8c@passwd.hu> <20211217112417.GG2829255@pb2> <5d14188-a4b0-4978-104c-59595fed8137@passwd.hu> <20211218133612.GL2829255@pb2> <20211218141516.GM2829255@pb2> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API 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 Sat, 18 Dec 2021, Michael Niedermayer wrote: > On Sat, Dec 18, 2021 at 02:36:12PM +0100, Michael Niedermayer wrote: >> On Fri, Dec 17, 2021 at 07:04:08PM +0100, Marton Balint wrote: >>> >>> >>> On Fri, 17 Dec 2021, Michael Niedermayer wrote: >>> >>>> On Fri, Dec 17, 2021 at 01:04:19AM +0100, Marton Balint wrote: >>>>> >>>>> >>>>> On Thu, 16 Dec 2021, James Almer wrote: >>>>> >>>>>> Resending the first two patches only, since this is meant to >>>>>> show the implementation of one of the several suggestions made >>>>>> in the previous set that need to be discussed and hopefully >>>>>> resolved in a call. >>>>> >>>>> Can you push the full branch somewhere? >>>>> >>>>>> >>>>>> The proposals so far to extend the API to support either custom >>>>>> labels for channels are, or some form of extra user information. >>>>>> >>>>>> - Fixed array of bytes to hold a label. Simple solution, but >>>>>> the labels will have a hard limit that can only be extended >>>>>> with a major bump. This is what i implemented in this version. >>>>>> - "char *name" per channel that the user may allocate and the >>>>>> API will manage, duplicate and free. Simple solution, and the >>>>>> name can be arbitrarily long, but inefficient (av_strdup() per >>>>>> channel with a custom label on layout copy). >>>>>> - "const char *name" per channel for compile time constants, or >>>>>> that the user may allocate and free. Very efficient, but for >>>>>> non compile time strings ensuring they outlive the layout can >>>>>> be tricky. >>>>>> - Refcounted AVChannelCustom with a dictionary. This can't be >>>>>> done with AVBufferRef, so it would require some other form >>>>>> of reference counting. And a dictionary may add quite a bit of >>>>>> complexity to the API, as you can set anything on them. >>>>> >>>>> Until we have proper refcounting API we can make the AVBufferRef in >>>>> AVChannelLayout a void *, and only allow channel_layout functions to >>>>> dereference it as an AVBufferRef. This would mean adding some extra helper >>>>> functions to channel layout, but overall it is not unsolvable. >>>>> >>>>> The real question is that if you want to use refcounting and add helpers to >>>>> query / replace per-channel metadata, or you find the idea too heavy weight >>>>> and would like to stick to flat structs. >>>> >>>> what is the advantage of refcounting for channel metadata ? >>>> is it about the used memory, about the reduced need to copy ? >>> >>> Basicly it is the ability to store per-channel metadata in avdictionary, >>> because otherwise it would have to be copyed, and avdictionary is very >>> ineffective at copying because of many mallocs. >>> >>>> >>>> what kind of metadata and what size do you expect ? >>>> bytes, kilobytes, megabytes, gigabytes per channel ? >>> >>> Usually, nothing, because most format don't have support for per-channel >>> metadata. In some cases it is going to be a couple of textual metadata >>> key-value pairs, such as language, label, group, speaker, positon, so 4-5 >>> dynamically allocated string pairs, plus the AVDictionary itself, multiplied >>> by the number of channels in a layout. >>> >>>> >>>> what is the overhead for dynamic allocation and ref counting? >>>> that is at which point does it even make sense ? >>> >>> I don't have exact measurements. It is generally felt that copying >>> AVDictionary per-channel is a huge overhead for something as lightweight as >>> an audio frame which is a 2-4 kB per channel at most and only a couple of >>> allocs usually not dependant on the number of channels. That's why >>> refcounting was proposed. >> >> I was thinking more at a AVStream / AVCodecParameters level. > >> How will a demuxer transport such metadata over a AVPacket into a decoder >> outputting metadata-filled AVFrames? > > or is this never needed ? I am not sure I understand. Usually metadata is passed from demuxer to decoder by avcodec_parameters_to_context(), this is used for all metadata which is in AVCodecParameters. For per-packet metadata ff_decode_frame_props() has some automatic packet side data -> frame side data transfer. AVStream side data may be transferred to AVPacket side data if av_format_inject_global_side_data() is used, but it is not enabled by default. 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-request@ffmpeg.org with subject "unsubscribe".