Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
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 01:04:19 +0100 (CET)
Message-ID: <92861361-f316-78a5-3c72-1e279d2a9f8c@passwd.hu> (raw)
In-Reply-To: <20211216132151.8216-1-jamrial@gmail.com>



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.

> - Opaque id/s or pointer/s that the API will not touch beyond
>  passing them around (So unlike the above, the helpers would not
>  benefit from this). This can be combined with any of the above,
>  too, and i did as much in this version.
> - Leave API as it was in v1.

Maybe it is not said enough times, but thanks to everybody who worked on 
this. It certainly was huge work, and I know that it is a thankless effort 
to get such a big change merged. Any change based on my suggestions is 
appreciated, even if some of my ideas get rejected in the end.

Thanks,
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".

  parent reply	other threads:[~2021-12-17  0: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 ` Marton Balint [this message]
2021-12-17  2:37   ` [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API 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
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=92861361-f316-78a5-3c72-1e279d2a9f8c@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