From: Marton Balint <cus@passwd.hu> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API Date: Fri, 17 Dec 2021 19:04:08 +0100 (CET) Message-ID: <5d14188-a4b0-4978-104c-59595fed8137@passwd.hu> (raw) In-Reply-To: <20211217112417.GG2829255@pb2> 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. Also some people simply don't want to store extendable channel metadata in channel layout, and want to keep it simple. 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".
next prev parent reply other threads:[~2021-12-17 18:04 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-16 13:21 James Almer 2021-12-16 13:21 ` [FFmpeg-devel] [PATCH 001/279 v2] Add a new " James Almer 2021-12-16 17:20 ` Paul B Mahol 2021-12-16 18:27 ` James Almer 2021-12-16 18:31 ` Paul B Mahol 2021-12-16 19:14 ` James Almer 2021-12-16 23:27 ` Marton Balint 2021-12-17 2:34 ` James Almer 2021-12-17 12:43 ` James Almer 2021-12-16 13:21 ` [FFmpeg-devel] [PATCH 002/279 v2] fate: add a channel_layout API test James Almer 2021-12-17 0:04 ` [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API Marton Balint 2021-12-17 2:37 ` James Almer 2021-12-17 19:20 ` Marton Balint 2021-12-17 19:32 ` James Almer 2021-12-17 11:24 ` Michael Niedermayer 2021-12-17 18:04 ` Marton Balint [this message] 2021-12-18 13:36 ` Michael Niedermayer 2021-12-18 14:15 ` Michael Niedermayer 2021-12-19 11:35 ` Marton Balint 2021-12-19 12:51 ` Michael Niedermayer
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5d14188-a4b0-4978-104c-59595fed8137@passwd.hu \ --to=cus@passwd.hu \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git