Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 11/10/19, Vittorio Giovara wrote: > On Mon, Oct 28, 2019 at 10:48 PM Paul B Mahol wrote: > >> The new API is more extensible and allows for custom layouts. >> More accurate information is exported, eg for decoders that do not >> set a channel layout, lavc will not make one up for them. >> >> Deprecate the old API working with just uint64_t bitmasks. >> >> Original commit by Anton Khirnov . >> Expanded and completed by Vittorio Giovara . >> Adapted for FFmpeg by Paul B Mahol . >> >> Signed-off-by: Anton Khirnov >> Signed-off-by: Vittorio Giovara >> Signed-off-by: Paul B Mahol >> --- >> > > Hello, > this seems to coming from an outdated branch. > > Please use https://git.khirnov.net/libav.git/log/?h=ambisonic2 as > reference: it contains several improvements from the last iteration, such > as a fully functional backward compatibility layer, and a reworked > implementation of ambisonics, and contains a fully rebased tree with all > ffmpeg formats and codecs. > > Really looking forward to getting this merged, thanks for your help in the > process. Only way to get this approved is sending it to this mailing list. > -- > Vittorio > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On Mon, Oct 28, 2019 at 10:48 PM Paul B Mahol wrote: > The new API is more extensible and allows for custom layouts. > More accurate information is exported, eg for decoders that do not > set a channel layout, lavc will not make one up for them. > > Deprecate the old API working with just uint64_t bitmasks. > > Original commit by Anton Khirnov . > Expanded and completed by Vittorio Giovara . > Adapted for FFmpeg by Paul B Mahol . > > Signed-off-by: Anton Khirnov > Signed-off-by: Vittorio Giovara > Signed-off-by: Paul B Mahol > --- > Hello, this seems to coming from an outdated branch. Please use https://git.khirnov.net/libav.git/log/?h=ambisonic2 as reference: it contains several improvements from the last iteration, such as a fully functional backward compatibility layer, and a reworked implementation of ambisonics, and contains a fully rebased tree with all ffmpeg formats and codecs. Really looking forward to getting this merged, thanks for your help in the process. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (12019-10-28):
> +char *av_channel_layout_describe(const AVChannelLayout *channel_layout)
> +{
> +int i;
> +
> +switch (channel_layout->order) {
> +case AV_CHANNEL_ORDER_NATIVE:
> +for (i = 0; channel_layout_map[i].name; i++)
> +if (channel_layout->u.mask ==
> channel_layout_map[i].layout.u.mask)
> +return av_strdup(channel_layout_map[i].name);
> +// fall-through
> +case AV_CHANNEL_ORDER_CUSTOM: {
> +// max 4 bytes for channel name + a separator
> +int size = 5 * channel_layout->nb_channels + 1;
> +char *ret;
> +
> +ret = av_mallocz(size);
> +if (!ret)
> +return NULL;
> +
> +for (i = 0; i < channel_layout->nb_channels; i++) {
> +enum AVChannel ch =
> av_channel_layout_get_channel(channel_layout, i);
> +const char *ch_name = av_channel_name(ch);
> +
> +if (i)
> +av_strlcat(ret, "+", size);
> +av_strlcat(ret, ch_name, size);
> +}
> +return ret;
> +}
We can do that with AVBPrint.
> +case AV_CHANNEL_ORDER_UNSPEC: {
> +char buf[64];
> +snprintf(buf, sizeof(buf), "%d channels",
> channel_layout->nb_channels);
> +return av_strdup(buf);
> +}
> +default:
> +return NULL;
> +}
> +}
What is the user interface to designate a specific channel in a channel
layout with duplicates?
For example, I could imagine an equalizer that splits each channel into
three (low, mid, high). Then, with pan :
pan=stereo|FL=0.5*FL0+0.8*FL1+1*FL2|FR=0.5*FR0+0.8*FR1+1*FR2
Unfortunately:
- The API has nothing to specify why there are three FL channels and
thee FR channels. The possibility to attach an arbitrary label for
custom layouts should be considered.
- We need something like
idx = av_channel_layout_get_channel_by_descr(cl, "FL1");
Note: Paul, please do not take these comments as attacks against your
work. You took work that was developed in libav and ported it here, that
is a worthy achievement. But at the time it was designed, libav had
about 1.5 active developers, and they have about a quarter of our
features, and therefore of our needs. With these circumstances, it is
normal they forgot a lot of things. Our goal should be to design an API
that will be convenient to use in the long run, not to integrate their
imperfect API as fast as possible.
Regards,
--
Nicolas George
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (12019-10-28):
> The new API is more extensible and allows for custom layouts.
> More accurate information is exported, eg for decoders that do not
> set a channel layout, lavc will not make one up for them.
>
> Deprecate the old API working with just uint64_t bitmasks.
>
> Original commit by Anton Khirnov .
> Expanded and completed by Vittorio Giovara .
> Adapted for FFmpeg by Paul B Mahol .
>
> Signed-off-by: Anton Khirnov
> Signed-off-by: Vittorio Giovara
> Signed-off-by: Paul B Mahol
> ---
> libavutil/channel_layout.c | 385 --
> libavutil/channel_layout.h | 411 ++---
> libavutil/version.h| 3 +
> 3 files changed, 709 insertions(+), 90 deletions(-)
Just a few quick remarks on the API itself for now. I did not read the
implementation carefully.
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index 3bd5ee29b7..9112af32a6 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -37,75 +37,87 @@ struct channel_name {
> };
>
> static const struct channel_name channel_names[] = {
> - [0] = { "FL","front left"},
> - [1] = { "FR","front right" },
> - [2] = { "FC","front center" },
> - [3] = { "LFE", "low frequency" },
> - [4] = { "BL","back left" },
> - [5] = { "BR","back right"},
> - [6] = { "FLC", "front left-of-center" },
> - [7] = { "FRC", "front right-of-center" },
> - [8] = { "BC","back center" },
> - [9] = { "SL","side left" },
> -[10] = { "SR","side right"},
> -[11] = { "TC","top center"},
> -[12] = { "TFL", "top front left"},
> -[13] = { "TFC", "top front center" },
> -[14] = { "TFR", "top front right" },
> -[15] = { "TBL", "top back left" },
> -[16] = { "TBC", "top back center" },
> -[17] = { "TBR", "top back right"},
> -[29] = { "DL","downmix left" },
> -[30] = { "DR","downmix right" },
> -[31] = { "WL","wide left" },
> -[32] = { "WR","wide right"},
> -[33] = { "SDL", "surround direct left" },
> -[34] = { "SDR", "surround direct right" },
> -[35] = { "LFE2", "low frequency 2" },
> +[AV_CHAN_FRONT_LEFT ] = { "FL" },
> +[AV_CHAN_FRONT_RIGHT ] = { "FR" },
> +[AV_CHAN_FRONT_CENTER] = { "FC" },
> +[AV_CHAN_LOW_FREQUENCY ] = { "LFE" },
> +[AV_CHAN_BACK_LEFT ] = { "BL" },
> +[AV_CHAN_BACK_RIGHT ] = { "BR" },
> +[AV_CHAN_FRONT_LEFT_OF_CENTER] = { "FLC" },
> +[AV_CHAN_FRONT_RIGHT_OF_CENTER ] = { "FRC" },
> +[AV_CHAN_BACK_CENTER ] = { "BC" },
> +[AV_CHAN_SIDE_LEFT ] = { "SL" },
> +[AV_CHAN_SIDE_RIGHT ] = { "SR" },
> +[AV_CHAN_TOP_CENTER ] = { "TC" },
> +[AV_CHAN_TOP_FRONT_LEFT ] = { "TFL" },
> +[AV_CHAN_TOP_FRONT_CENTER] = { "TFC" },
> +[AV_CHAN_TOP_FRONT_RIGHT ] = { "TFR" },
> +[AV_CHAN_TOP_BACK_LEFT ] = { "TBL" },
> +[AV_CHAN_TOP_BACK_CENTER ] = { "TBC" },
> +[AV_CHAN_TOP_BACK_RIGHT ] = { "TBR" },
> +[AV_CHAN_STEREO_LEFT ] = { "DL" },
> +[AV_CHAN_STEREO_RIGHT] = { "DR" },
> +[AV_CHAN_WIDE_LEFT ] = { "WL" },
> +[AV_CHAN_WIDE_RIGHT ] = { "WR" },
> +[AV_CHAN_SURROUND_DIRECT_LEFT] = { "SDL" },
> +[AV_CHAN_SURROUND_DIRECT_RIGHT ] = { "SDR" },
> +[AV_CHAN_LOW_FREQUENCY_2 ] = { "LFE2" },
> +[AV_CHAN_SILENCE ] = { "PAD" },
> };
>
> -static const char *get_channel_name(int channel_id)
> +const char *av_channel_name(enum AVChannel channel_id)
> {
> if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
> return NULL;
> return channel_names[channel_id].name;
> }
>
> +int av_channel_from_string(const char *str)
> +{
> +for (int i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
> +if (channel_names[i].name && !strcmp(str, channel_names[i].name)) {
> +return i;
> +}
> +}
> +return AVERROR(EINVAL);
> +}
> +
> static const struct {
> const char *name;
> -int nb_channels;
> -uint64_t layout;
> +AVChannelLayout layout;
> } channel_layout_map[] = {
> -{ "mono",1, AV_CH_LAYOUT_MONO },
> -{ "stereo", 2, AV_CH_LAYOUT_STEREO },
> -{ "2.1", 3, AV_CH_LAYOUT_2POINT1 },
> -{ "3.0", 3, AV_CH_LAYOUT_SURROUND },
> -{ "3.0(back)", 3, AV_CH_LAYOUT_2_1 },
> -{ "4.0", 4, AV_CH_LAYOUT_4POINT0
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On Mon, Oct 28, 2019 at 02:48:21PM +0100, Paul B Mahol wrote:
> The new API is more extensible and allows for custom layouts.
> More accurate information is exported, eg for decoders that do not
> set a channel layout, lavc will not make one up for them.
>
> Deprecate the old API working with just uint64_t bitmasks.
>
> Original commit by Anton Khirnov .
> Expanded and completed by Vittorio Giovara .
> Adapted for FFmpeg by Paul B Mahol .
>
> Signed-off-by: Anton Khirnov
> Signed-off-by: Vittorio Giovara
> Signed-off-by: Paul B Mahol
> ---
> libavutil/channel_layout.c | 385 --
> libavutil/channel_layout.h | 411 ++---
> libavutil/version.h| 3 +
> 3 files changed, 709 insertions(+), 90 deletions(-)
>
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index 3bd5ee29b7..9112af32a6 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
[...]
./ffmpeg -v 0 -layouts
looses the channel names
[...]
> +void av_channel_layout_from_mask(AVChannelLayout *channel_layout,
> + uint64_t mask)
> +{
> +channel_layout->order = AV_CHANNEL_ORDER_NATIVE;
> +channel_layout->nb_channels = av_popcount64(mask);
is this correct in relation to AV_CH_LAYOUT_NATIVE ?
> +channel_layout->u.mask = mask;
> +}
[...]
> +/**
> + * Initialize a native channel layout from a bitmask indicating which
> channels
> + * are present.
> + *
> + * @note channel_layout should be properly allocated as described above.
> + *
> + * @param channel_layout the layout structure to be initialized
> + * @param mask bitmask describing the channel layout
> + */
> +void av_channel_layout_from_mask(AVChannelLayout *channel_layout, uint64_t
> mask);
This possibly should return a error code
for example a mask of 0 would not be valid
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I have often repented speaking, but never of holding my tongue.
-- Xenocrates
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] Add a new channel layout API
The new API is more extensible and allows for custom layouts.
More accurate information is exported, eg for decoders that do not
set a channel layout, lavc will not make one up for them.
Deprecate the old API working with just uint64_t bitmasks.
Original commit by Anton Khirnov .
Expanded and completed by Vittorio Giovara .
Adapted for FFmpeg by Paul B Mahol .
Signed-off-by: Anton Khirnov
Signed-off-by: Vittorio Giovara
Signed-off-by: Paul B Mahol
---
libavutil/channel_layout.c | 385 --
libavutil/channel_layout.h | 411 ++---
libavutil/version.h| 3 +
3 files changed, 709 insertions(+), 90 deletions(-)
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 3bd5ee29b7..9112af32a6 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -37,75 +37,87 @@ struct channel_name {
};
static const struct channel_name channel_names[] = {
- [0] = { "FL","front left"},
- [1] = { "FR","front right" },
- [2] = { "FC","front center" },
- [3] = { "LFE", "low frequency" },
- [4] = { "BL","back left" },
- [5] = { "BR","back right"},
- [6] = { "FLC", "front left-of-center" },
- [7] = { "FRC", "front right-of-center" },
- [8] = { "BC","back center" },
- [9] = { "SL","side left" },
-[10] = { "SR","side right"},
-[11] = { "TC","top center"},
-[12] = { "TFL", "top front left"},
-[13] = { "TFC", "top front center" },
-[14] = { "TFR", "top front right" },
-[15] = { "TBL", "top back left" },
-[16] = { "TBC", "top back center" },
-[17] = { "TBR", "top back right"},
-[29] = { "DL","downmix left" },
-[30] = { "DR","downmix right" },
-[31] = { "WL","wide left" },
-[32] = { "WR","wide right"},
-[33] = { "SDL", "surround direct left" },
-[34] = { "SDR", "surround direct right" },
-[35] = { "LFE2", "low frequency 2" },
+[AV_CHAN_FRONT_LEFT ] = { "FL" },
+[AV_CHAN_FRONT_RIGHT ] = { "FR" },
+[AV_CHAN_FRONT_CENTER] = { "FC" },
+[AV_CHAN_LOW_FREQUENCY ] = { "LFE" },
+[AV_CHAN_BACK_LEFT ] = { "BL" },
+[AV_CHAN_BACK_RIGHT ] = { "BR" },
+[AV_CHAN_FRONT_LEFT_OF_CENTER] = { "FLC" },
+[AV_CHAN_FRONT_RIGHT_OF_CENTER ] = { "FRC" },
+[AV_CHAN_BACK_CENTER ] = { "BC" },
+[AV_CHAN_SIDE_LEFT ] = { "SL" },
+[AV_CHAN_SIDE_RIGHT ] = { "SR" },
+[AV_CHAN_TOP_CENTER ] = { "TC" },
+[AV_CHAN_TOP_FRONT_LEFT ] = { "TFL" },
+[AV_CHAN_TOP_FRONT_CENTER] = { "TFC" },
+[AV_CHAN_TOP_FRONT_RIGHT ] = { "TFR" },
+[AV_CHAN_TOP_BACK_LEFT ] = { "TBL" },
+[AV_CHAN_TOP_BACK_CENTER ] = { "TBC" },
+[AV_CHAN_TOP_BACK_RIGHT ] = { "TBR" },
+[AV_CHAN_STEREO_LEFT ] = { "DL" },
+[AV_CHAN_STEREO_RIGHT] = { "DR" },
+[AV_CHAN_WIDE_LEFT ] = { "WL" },
+[AV_CHAN_WIDE_RIGHT ] = { "WR" },
+[AV_CHAN_SURROUND_DIRECT_LEFT] = { "SDL" },
+[AV_CHAN_SURROUND_DIRECT_RIGHT ] = { "SDR" },
+[AV_CHAN_LOW_FREQUENCY_2 ] = { "LFE2" },
+[AV_CHAN_SILENCE ] = { "PAD" },
};
-static const char *get_channel_name(int channel_id)
+const char *av_channel_name(enum AVChannel channel_id)
{
if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
return NULL;
return channel_names[channel_id].name;
}
+int av_channel_from_string(const char *str)
+{
+for (int i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
+if (channel_names[i].name && !strcmp(str, channel_names[i].name)) {
+return i;
+}
+}
+return AVERROR(EINVAL);
+}
+
static const struct {
const char *name;
-int nb_channels;
-uint64_t layout;
+AVChannelLayout layout;
} channel_layout_map[] = {
-{ "mono",1, AV_CH_LAYOUT_MONO },
-{ "stereo", 2, AV_CH_LAYOUT_STEREO },
-{ "2.1", 3, AV_CH_LAYOUT_2POINT1 },
-{ "3.0", 3, AV_CH_LAYOUT_SURROUND },
-{ "3.0(back)", 3, AV_CH_LAYOUT_2_1 },
-{ "4.0", 4, AV_CH_LAYOUT_4POINT0 },
-{ "quad",4, AV_CH_LAYOUT_QUAD },
-{ "quad(side)", 4, AV_CH_LAYOUT_2_2 },
-{ "3.1", 4, AV_CH_LAYOUT_3POINT1 },
-{ "5.0", 5, AV_CH_LAYOUT_5POINT0_BACK },
-{ "5.0(side)", 5, AV_CH_LAYOUT_5POINT0 },
-{ "4.1", 5, AV_CH_LAYOUT_4POINT1 },
-{ "5.1", 6,
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/30/18, Nicolas George wrote: > Paul B Mahol (2018-12-30): >> I will use +. > > That was my suggestion in the beginning, so I am obviously for. But what > happened to your reasons for rejecting it? I thought that adapted patch by me had this issue already resolved. I did not carefully read full patch. Anyway I would spot this issue for sure at later point. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-30): > I will use +. That was my suggestion in the beginning, so I am obviously for. But what happened to your reasons for rejecting it? (Once again, too little information in your mail => wasted time for both of us.) -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/30/18, Nicolas George wrote: > Paul B Mahol (2018-12-30): >> http://ffmpeg.org/ffmpeg-filters.html#aformat-1http://ffmpeg.org/ffmpeg-filters.html#aformat-1 > > Your link was slightly broken. > >> Do you want to change above thing too? > > No, but I would have objected if I had noticed at the time. > > But thanks for finding that example: that shows that | cannot be used in > channel layout descriptions, since aformat splits on |. You have to use > another character, there is no choice about it. I will use +. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-30): > http://ffmpeg.org/ffmpeg-filters.html#aformat-1http://ffmpeg.org/ffmpeg-filters.html#aformat-1 Your link was slightly broken. > Do you want to change above thing too? No, but I would have objected if I had noticed at the time. But thanks for finding that example: that shows that | cannot be used in channel layout descriptions, since aformat splits on |. You have to use another character, there is no choice about it. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/30/18, Nicolas George wrote: > Paul B Mahol (2018-12-30): >> aformat for example use | to split channel layouts. > > Where? http://ffmpeg.org/ffmpeg-filters.html#aformat-1http://ffmpeg.org/ffmpeg-filters.html#aformat-1 Do you want to change above thing too? What you propose instead? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-30): > aformat for example use | to split channel layouts. Where? > Documentation follows old legacy usage. I am talking about the documentation of the patch you are proposing. > Can you please stop bikeshedding and arguing with me just because > you enjoy it? Can you refrain from making baseless accusations? I am concerned about the quality of a new API. Mistakes done now will bother us for a long time; it is therefore a good idea to take the time to think things through. Please assume that everybody has at least as much goodwill as yourself. If you want the discussion to go faster, I would suggest you change your habit of giving half a bit of information per mail: explain your point in details, with examples, and you will need to do it only once. If you keep this style of posting, each question will take a dozen mails: you are wasting your own time like that as well as the time of everybody else. -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/30/18, Nicolas George wrote: > Paul B Mahol (2018-12-30): >> Please no, | is used in bunch of scripts. > > What scripts? How can they use an API that is still in early discussion? aformat for example use | to split channel layouts. > > Also, discussing this is moot until you rephrase the documentation. Documentation follows old legacy usage. Can you please stop bikeshedding and arguing with me just because you enjoy it? I'm kindly asking you, whichever agenda you follow, please stop. I'm doing this to add support for ambisonics and you just stopped it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-30): > Please no, | is used in bunch of scripts. What scripts? How can they use an API that is still in early discussion? Also, discussing this is moot until you rephrase the documentation. -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/30/18, Nicolas George wrote: > Paul B Mahol (2018-12-30): >> >> Input #0, wv, from >> >> '/home/computer/Downloads/Run_The_Race_-_3rd_Order_Ambisonic_SN3D.wv': >> >> Duration: 00:11:07.67, start: 0.00, bitrate: 6084 kb/s >> >> Stream #0:0: Audio: wavpack, 44100 Hz, 16 channels >> >> (FL+FR+FC+LFE+BL+BR+FLC+FRC+BC+SL+SR+TC+TFL+TFC+TFR+TBL), s16p >> > >> > Yes... and...? >> >> From this is obvious that user can not use '+' to separate channel >> layouts if it is already used to define channel layouts. >> >> Do you need better explanation now? > > Well, quoting the original patch: > > + * The input string can be represented by: > + * - the formal channel layout name (returned by > av_channel_layout_describe()) > + * - single or multiple channel names (returned by av_channel_name() > + *or concatenated with "|") > + * - a hexadecimal value of a channel layout (eg. "0x4") > + * - the number of channels with default layout (eg. "5") > + * - the number of unordered channels (eg. "4 channels") > > There is no mention about '+'. So I guess this paragraph needs to be > rewritten to actually match what is being done. > > And if + is not possible, | is still a bad choice. Use a character that > is not special for standard shells, I am sure we can find some; maybe /. > Please no, | is used in bunch of scripts. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-30): > >> Input #0, wv, from > >> '/home/computer/Downloads/Run_The_Race_-_3rd_Order_Ambisonic_SN3D.wv': > >> Duration: 00:11:07.67, start: 0.00, bitrate: 6084 kb/s > >> Stream #0:0: Audio: wavpack, 44100 Hz, 16 channels > >> (FL+FR+FC+LFE+BL+BR+FLC+FRC+BC+SL+SR+TC+TFL+TFC+TFR+TBL), s16p > > > > Yes... and...? > > From this is obvious that user can not use '+' to separate channel > layouts if it is already used to define channel layouts. > > Do you need better explanation now? Well, quoting the original patch: + * The input string can be represented by: + * - the formal channel layout name (returned by av_channel_layout_describe()) + * - single or multiple channel names (returned by av_channel_name() + *or concatenated with "|") + * - a hexadecimal value of a channel layout (eg. "0x4") + * - the number of channels with default layout (eg. "5") + * - the number of unordered channels (eg. "4 channels") There is no mention about '+'. So I guess this paragraph needs to be rewritten to actually match what is being done. And if + is not possible, | is still a bad choice. Use a character that is not special for standard shells, I am sure we can find some; maybe /. -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/30/18, Nicolas George wrote: > Paul B Mahol (2018-12-30): >> >> > av_get_channel_layout() used to use '+' instead of '|', and I think >> >> > it >> >> > is better. For once, '+' is not a special character for shells. >> >> >> >> Can not work if user wants to define custom channel layout from >> >> already available channels. >> > >> > Please explain why. >> >> Input #0, wv, from >> '/home/computer/Downloads/Run_The_Race_-_3rd_Order_Ambisonic_SN3D.wv': >> Duration: 00:11:07.67, start: 0.00, bitrate: 6084 kb/s >> Stream #0:0: Audio: wavpack, 44100 Hz, 16 channels >> (FL+FR+FC+LFE+BL+BR+FLC+FRC+BC+SL+SR+TC+TFL+TFC+TFR+TBL), s16p > > Yes... and...? From this is obvious that user can not use '+' to separate channel layouts if it is already used to define channel layouts. Do you need better explanation now? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-30): > >> > av_get_channel_layout() used to use '+' instead of '|', and I think it > >> > is better. For once, '+' is not a special character for shells. > >> > >> Can not work if user wants to define custom channel layout from > >> already available channels. > > > > Please explain why. > > Input #0, wv, from > '/home/computer/Downloads/Run_The_Race_-_3rd_Order_Ambisonic_SN3D.wv': > Duration: 00:11:07.67, start: 0.00, bitrate: 6084 kb/s > Stream #0:0: Audio: wavpack, 44100 Hz, 16 channels > (FL+FR+FC+LFE+BL+BR+FLC+FRC+BC+SL+SR+TC+TFL+TFC+TFR+TBL), s16p Yes... and...? signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/30/18, Nicolas George wrote: > Paul B Mahol (2018-12-30): >> On 12/26/18, Nicolas George wrote: >> >> + * The input string can be represented by: >> >> + * - the formal channel layout name (returned by >> >> av_channel_layout_describe()) >> >> + * - single or multiple channel names (returned by av_channel_name() >> >> + *or concatenated with "|") >> >> + * - a hexadecimal value of a channel layout (eg. "0x4") >> >> + * - the number of channels with default layout (eg. "5") >> >> + * - the number of unordered channels (eg. "4 channels") >> > >> > av_get_channel_layout() used to use '+' instead of '|', and I think it >> > is better. For once, '+' is not a special character for shells. >> >> Can not work if user wants to define custom channel layout from >> already available channels. > > Please explain why. Input #0, wv, from '/home/computer/Downloads/Run_The_Race_-_3rd_Order_Ambisonic_SN3D.wv': Duration: 00:11:07.67, start: 0.00, bitrate: 6084 kb/s Stream #0:0: Audio: wavpack, 44100 Hz, 16 channels (FL+FR+FC+LFE+BL+BR+FLC+FRC+BC+SL+SR+TC+TFL+TFC+TFR+TBL), s16p ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-30): > On 12/26/18, Nicolas George wrote: > >> + * The input string can be represented by: > >> + * - the formal channel layout name (returned by > >> av_channel_layout_describe()) > >> + * - single or multiple channel names (returned by av_channel_name() > >> + *or concatenated with "|") > >> + * - a hexadecimal value of a channel layout (eg. "0x4") > >> + * - the number of channels with default layout (eg. "5") > >> + * - the number of unordered channels (eg. "4 channels") > > > > av_get_channel_layout() used to use '+' instead of '|', and I think it > > is better. For once, '+' is not a special character for shells. > > Can not work if user wants to define custom channel layout from > already available channels. Please explain why. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/26/18, Nicolas George wrote: >> + * The input string can be represented by: >> + * - the formal channel layout name (returned by >> av_channel_layout_describe()) >> + * - single or multiple channel names (returned by av_channel_name() >> + *or concatenated with "|") >> + * - a hexadecimal value of a channel layout (eg. "0x4") >> + * - the number of channels with default layout (eg. "5") >> + * - the number of unordered channels (eg. "4 channels") > > av_get_channel_layout() used to use '+' instead of '|', and I think it > is better. For once, '+' is not a special character for shells. Can not work if user wants to define custom channel layout from already available channels. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/26/2018 4:35 PM, Paul B Mahol wrote: > On 12/26/18, James Almer wrote: >> On 12/26/2018 12:07 PM, Paul B Mahol wrote: >>> On 12/26/18, Nicolas George wrote: > + > +/** > + * Initialize a native channel layout from a bitmask indicating which > channels > + * are present. > + * > + * @note channel_layout should be properly allocated as described > above. > + * > + * @param channel_layout the layout structure to be initialized > + * @param mask bitmask describing the channel layout > + */ > +void av_channel_layout_from_mask(AVChannelLayout *channel_layout, > uint64_t mask); > + > +/** > + * Initialize a channel layout from a given string description. > + * The input string can be represented by: > + * - the formal channel layout name (returned by > av_channel_layout_describe()) > + * - single or multiple channel names (returned by av_channel_name() > + *or concatenated with "|") > + * - a hexadecimal value of a channel layout (eg. "0x4") > + * - the number of channels with default layout (eg. "5") > + * - the number of unordered channels (eg. "4 channels") av_get_channel_layout() used to use '+' instead of '|', and I think it is better. For once, '+' is not a special character for shells. >>> >>> Look folk, I'm not paid to do this nor I'm paid to read your "reviews" >>> so I will ignore this one. >> What prompted you to reply this way? Was there a need to be this >> aggressive with a review? >> >> What do you or anyone wins with this? > > You called for this, I'm not gonna continue working on this. > > All thanks to very "nice" reviewers like all of you. You're making no sense... ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/26/18, James Almer wrote: > On 12/26/2018 12:07 PM, Paul B Mahol wrote: >> On 12/26/18, Nicolas George wrote: + +/** + * Initialize a native channel layout from a bitmask indicating which channels + * are present. + * + * @note channel_layout should be properly allocated as described above. + * + * @param channel_layout the layout structure to be initialized + * @param mask bitmask describing the channel layout + */ +void av_channel_layout_from_mask(AVChannelLayout *channel_layout, uint64_t mask); + +/** + * Initialize a channel layout from a given string description. >>> + * The input string can be represented by: + * - the formal channel layout name (returned by av_channel_layout_describe()) + * - single or multiple channel names (returned by av_channel_name() + *or concatenated with "|") + * - a hexadecimal value of a channel layout (eg. "0x4") + * - the number of channels with default layout (eg. "5") + * - the number of unordered channels (eg. "4 channels") >>> >>> av_get_channel_layout() used to use '+' instead of '|', and I think it >>> is better. For once, '+' is not a special character for shells. >> >> Look folk, I'm not paid to do this nor I'm paid to read your "reviews" >> so I will ignore this one. > What prompted you to reply this way? Was there a need to be this > aggressive with a review? > > What do you or anyone wins with this? You called for this, I'm not gonna continue working on this. All thanks to very "nice" reviewers like all of you. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/26/2018 12:07 PM, Paul B Mahol wrote: > On 12/26/18, Nicolas George wrote: >>> + >>> +/** >>> + * Initialize a native channel layout from a bitmask indicating which >>> channels >>> + * are present. >>> + * >>> + * @note channel_layout should be properly allocated as described above. >>> + * >>> + * @param channel_layout the layout structure to be initialized >>> + * @param mask bitmask describing the channel layout >>> + */ >>> +void av_channel_layout_from_mask(AVChannelLayout *channel_layout, >>> uint64_t mask); >>> + >>> +/** >>> + * Initialize a channel layout from a given string description. >> >>> + * The input string can be represented by: >>> + * - the formal channel layout name (returned by >>> av_channel_layout_describe()) >>> + * - single or multiple channel names (returned by av_channel_name() >>> + *or concatenated with "|") >>> + * - a hexadecimal value of a channel layout (eg. "0x4") >>> + * - the number of channels with default layout (eg. "5") >>> + * - the number of unordered channels (eg. "4 channels") >> >> av_get_channel_layout() used to use '+' instead of '|', and I think it >> is better. For once, '+' is not a special character for shells. > > Look folk, I'm not paid to do this nor I'm paid to read your "reviews" > so I will ignore this one. What prompted you to reply this way? Was there a need to be this aggressive with a review? What do you or anyone wins with this? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-26): > Look folk, I'm not paid to do this nor I'm paid to read your "reviews" Neither am I. What does it have to do with anything? > so I will ignore this one. You are not allowed to do that. FFmpeg is not your personal project, it is a collective endeavour governed by consensus. This patch cannot go in until consensus is reached, and consensus cannot be reached unless you take reviews into account. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/26/18, Nicolas George wrote:
> Paul B Mahol (2018-12-24):
>> The new API is more extensible and allows for custom layouts.
>> More accurate information is exported, eg for decoders that do not
>> set a channel layout, lavc will not make one up for them.
>>
>> Deprecate the old API working with just uint64_t bitmasks.
>>
>> Original commit by Anton Khirnov .
>> Expanded and completed by Vittorio Giovara .
>
>> Adopted for FFmpeg by Paul B Mahol .
>
> Adapted?
>
> Reviewing only the visible API for now.
>
>> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
>> index 50bb8f03c5..e6c8c58b9c 100644
>> --- a/libavutil/channel_layout.h
>> +++ b/libavutil/channel_layout.h
>> @@ -24,6 +24,9 @@
>>
>> #include
>>
>> +#include "version.h"
>> +#include "attributes.h"
>> +
>> /**
>> * @file
>> * audio channel layout utility functions
>> @@ -34,6 +37,60 @@
>> * @{
>> */
>>
>> +enum AVChannel {
>> +AV_CHAN_FRONT_LEFT,
>> +AV_CHAN_FRONT_RIGHT,
>> +AV_CHAN_FRONT_CENTER,
>> +AV_CHAN_LOW_FREQUENCY,
>> +AV_CHAN_BACK_LEFT,
>> +AV_CHAN_BACK_RIGHT,
>> +AV_CHAN_FRONT_LEFT_OF_CENTER,
>> +AV_CHAN_FRONT_RIGHT_OF_CENTER,
>> +AV_CHAN_BACK_CENTER,
>> +AV_CHAN_SIDE_LEFT,
>> +AV_CHAN_SIDE_RIGHT,
>> +AV_CHAN_TOP_CENTER,
>> +AV_CHAN_TOP_FRONT_LEFT,
>> +AV_CHAN_TOP_FRONT_CENTER,
>> +AV_CHAN_TOP_FRONT_RIGHT,
>> +AV_CHAN_TOP_BACK_LEFT,
>> +AV_CHAN_TOP_BACK_CENTER,
>> +AV_CHAN_TOP_BACK_RIGHT,
>> +/** Stereo downmix. */
>> +AV_CHAN_STEREO_LEFT = 29,
>> +/** See above. */
>> +AV_CHAN_STEREO_RIGHT,
>> +AV_CHAN_WIDE_LEFT,
>> +AV_CHAN_WIDE_RIGHT,
>> +AV_CHAN_SURROUND_DIRECT_LEFT,
>> +AV_CHAN_SURROUND_DIRECT_RIGHT,
>> +AV_CHAN_LOW_FREQUENCY_2,
>> +
>> +/** Channel is empty can be safely skipped. */
>> +AV_CHAN_SILENCE = 64,
>> +};
>> +
>> +enum AVChannelOrder {
>> +/**
>> + * The native channel order, i.e. the channels are in the same order
>> in
>> + * which they are defined in the AVChannel enum. This supports up to
>> 63
>> + * different channels.
>> + */
>> +AV_CHANNEL_ORDER_NATIVE,
>> +/**
>> + * The channel order does not correspond to any other predefined
>> order and
>> + * is stored as an explicit map. For example, this could be used to
>> support
>> + * layouts with 64 or more channels, or with channels that could be
>> skipped.
>> + */
>> +AV_CHANNEL_ORDER_CUSTOM,
>> +/**
>> + * Only the channel count is specified, without any further
>> information
>> + * about the channel order.
>> + */
>> +AV_CHANNEL_ORDER_UNSPEC,
>> +};
>> +
>> +
>> /**
>> * @defgroup channel_masks Audio channel masks
>> *
>> @@ -46,36 +103,41 @@
>> *
>> * @{
>> */
>> -#define AV_CH_FRONT_LEFT 0x0001
>> -#define AV_CH_FRONT_RIGHT0x0002
>> -#define AV_CH_FRONT_CENTER 0x0004
>> -#define AV_CH_LOW_FREQUENCY 0x0008
>> -#define AV_CH_BACK_LEFT 0x0010
>> -#define AV_CH_BACK_RIGHT 0x0020
>> -#define AV_CH_FRONT_LEFT_OF_CENTER 0x0040
>> -#define AV_CH_FRONT_RIGHT_OF_CENTER 0x0080
>> -#define AV_CH_BACK_CENTER0x0100
>> -#define AV_CH_SIDE_LEFT 0x0200
>> -#define AV_CH_SIDE_RIGHT 0x0400
>> -#define AV_CH_TOP_CENTER 0x0800
>> -#define AV_CH_TOP_FRONT_LEFT 0x1000
>> -#define AV_CH_TOP_FRONT_CENTER 0x2000
>> -#define AV_CH_TOP_FRONT_RIGHT0x4000
>> -#define AV_CH_TOP_BACK_LEFT 0x8000
>> -#define AV_CH_TOP_BACK_CENTER0x0001
>> -#define AV_CH_TOP_BACK_RIGHT 0x0002
>> -#define AV_CH_STEREO_LEFT0x2000 ///< Stereo downmix.
>> -#define AV_CH_STEREO_RIGHT 0x4000 ///< See
>> AV_CH_STEREO_LEFT.
>> -#define AV_CH_WIDE_LEFT 0x8000ULL
>> -#define AV_CH_WIDE_RIGHT 0x0001ULL
>> -#define AV_CH_SURROUND_DIRECT_LEFT 0x0002ULL
>> -#define AV_CH_SURROUND_DIRECT_RIGHT 0x0004ULL
>> -#define AV_CH_LOW_FREQUENCY_20x0008ULL
>> +#define AV_CH_FRONT_LEFT (1ULL << AV_CHAN_FRONT_LEFT
>> )
>> +#define AV_CH_FRONT_RIGHT(1ULL << AV_CHAN_FRONT_RIGHT
>> )
>> +#define AV_CH_FRONT_CENTER (1ULL << AV_CHAN_FRONT_CENTER
>> )
>> +#define AV_CH_LOW_FREQUENCY (1ULL << AV_CHAN_LOW_FREQUENCY
>> )
>> +#define AV_CH_BACK_LEFT (1ULL << AV_CHAN_BACK_LEFT
>> )
>> +#define AV_CH_BACK_RIGHT (1ULL << AV_CHAN_BACK_RIGHT
>> )
>> +#define AV_CH_FRONT_LEFT_OF_CENTER (1ULL <<
>> AV_CHAN_FRONT_LEFT_OF_CENTER )
>> +#define AV_CH_FRONT_RIGHT_OF_CENTER (1ULL <<
>> AV_CHAN_FRONT_RIGHT_OF_CENTER)
>> +#define AV_CH_BACK_CENTER(1ULL << AV_CHAN_BACK_CENTER
>> )
>> +#define AV_CH_SIDE_LEFT (1ULL << AV_CHAN_SIDE_LEFT
>> )
>> +#define AV_CH_SIDE_RIGHT
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
Paul B Mahol (2018-12-24):
> The new API is more extensible and allows for custom layouts.
> More accurate information is exported, eg for decoders that do not
> set a channel layout, lavc will not make one up for them.
>
> Deprecate the old API working with just uint64_t bitmasks.
>
> Original commit by Anton Khirnov .
> Expanded and completed by Vittorio Giovara .
> Adopted for FFmpeg by Paul B Mahol .
Adapted?
Reviewing only the visible API for now.
> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
> index 50bb8f03c5..e6c8c58b9c 100644
> --- a/libavutil/channel_layout.h
> +++ b/libavutil/channel_layout.h
> @@ -24,6 +24,9 @@
>
> #include
>
> +#include "version.h"
> +#include "attributes.h"
> +
> /**
> * @file
> * audio channel layout utility functions
> @@ -34,6 +37,60 @@
> * @{
> */
>
> +enum AVChannel {
> +AV_CHAN_FRONT_LEFT,
> +AV_CHAN_FRONT_RIGHT,
> +AV_CHAN_FRONT_CENTER,
> +AV_CHAN_LOW_FREQUENCY,
> +AV_CHAN_BACK_LEFT,
> +AV_CHAN_BACK_RIGHT,
> +AV_CHAN_FRONT_LEFT_OF_CENTER,
> +AV_CHAN_FRONT_RIGHT_OF_CENTER,
> +AV_CHAN_BACK_CENTER,
> +AV_CHAN_SIDE_LEFT,
> +AV_CHAN_SIDE_RIGHT,
> +AV_CHAN_TOP_CENTER,
> +AV_CHAN_TOP_FRONT_LEFT,
> +AV_CHAN_TOP_FRONT_CENTER,
> +AV_CHAN_TOP_FRONT_RIGHT,
> +AV_CHAN_TOP_BACK_LEFT,
> +AV_CHAN_TOP_BACK_CENTER,
> +AV_CHAN_TOP_BACK_RIGHT,
> +/** Stereo downmix. */
> +AV_CHAN_STEREO_LEFT = 29,
> +/** See above. */
> +AV_CHAN_STEREO_RIGHT,
> +AV_CHAN_WIDE_LEFT,
> +AV_CHAN_WIDE_RIGHT,
> +AV_CHAN_SURROUND_DIRECT_LEFT,
> +AV_CHAN_SURROUND_DIRECT_RIGHT,
> +AV_CHAN_LOW_FREQUENCY_2,
> +
> +/** Channel is empty can be safely skipped. */
> +AV_CHAN_SILENCE = 64,
> +};
> +
> +enum AVChannelOrder {
> +/**
> + * The native channel order, i.e. the channels are in the same order in
> + * which they are defined in the AVChannel enum. This supports up to 63
> + * different channels.
> + */
> +AV_CHANNEL_ORDER_NATIVE,
> +/**
> + * The channel order does not correspond to any other predefined order
> and
> + * is stored as an explicit map. For example, this could be used to
> support
> + * layouts with 64 or more channels, or with channels that could be
> skipped.
> + */
> +AV_CHANNEL_ORDER_CUSTOM,
> +/**
> + * Only the channel count is specified, without any further information
> + * about the channel order.
> + */
> +AV_CHANNEL_ORDER_UNSPEC,
> +};
> +
> +
> /**
> * @defgroup channel_masks Audio channel masks
> *
> @@ -46,36 +103,41 @@
> *
> * @{
> */
> -#define AV_CH_FRONT_LEFT 0x0001
> -#define AV_CH_FRONT_RIGHT0x0002
> -#define AV_CH_FRONT_CENTER 0x0004
> -#define AV_CH_LOW_FREQUENCY 0x0008
> -#define AV_CH_BACK_LEFT 0x0010
> -#define AV_CH_BACK_RIGHT 0x0020
> -#define AV_CH_FRONT_LEFT_OF_CENTER 0x0040
> -#define AV_CH_FRONT_RIGHT_OF_CENTER 0x0080
> -#define AV_CH_BACK_CENTER0x0100
> -#define AV_CH_SIDE_LEFT 0x0200
> -#define AV_CH_SIDE_RIGHT 0x0400
> -#define AV_CH_TOP_CENTER 0x0800
> -#define AV_CH_TOP_FRONT_LEFT 0x1000
> -#define AV_CH_TOP_FRONT_CENTER 0x2000
> -#define AV_CH_TOP_FRONT_RIGHT0x4000
> -#define AV_CH_TOP_BACK_LEFT 0x8000
> -#define AV_CH_TOP_BACK_CENTER0x0001
> -#define AV_CH_TOP_BACK_RIGHT 0x0002
> -#define AV_CH_STEREO_LEFT0x2000 ///< Stereo downmix.
> -#define AV_CH_STEREO_RIGHT 0x4000 ///< See AV_CH_STEREO_LEFT.
> -#define AV_CH_WIDE_LEFT 0x8000ULL
> -#define AV_CH_WIDE_RIGHT 0x0001ULL
> -#define AV_CH_SURROUND_DIRECT_LEFT 0x0002ULL
> -#define AV_CH_SURROUND_DIRECT_RIGHT 0x0004ULL
> -#define AV_CH_LOW_FREQUENCY_20x0008ULL
> +#define AV_CH_FRONT_LEFT (1ULL << AV_CHAN_FRONT_LEFT )
> +#define AV_CH_FRONT_RIGHT(1ULL << AV_CHAN_FRONT_RIGHT )
> +#define AV_CH_FRONT_CENTER (1ULL << AV_CHAN_FRONT_CENTER )
> +#define AV_CH_LOW_FREQUENCY (1ULL << AV_CHAN_LOW_FREQUENCY)
> +#define AV_CH_BACK_LEFT (1ULL << AV_CHAN_BACK_LEFT)
> +#define AV_CH_BACK_RIGHT (1ULL << AV_CHAN_BACK_RIGHT )
> +#define AV_CH_FRONT_LEFT_OF_CENTER (1ULL << AV_CHAN_FRONT_LEFT_OF_CENTER )
> +#define AV_CH_FRONT_RIGHT_OF_CENTER (1ULL << AV_CHAN_FRONT_RIGHT_OF_CENTER)
> +#define AV_CH_BACK_CENTER(1ULL << AV_CHAN_BACK_CENTER )
> +#define AV_CH_SIDE_LEFT (1ULL << AV_CHAN_SIDE_LEFT)
> +#define AV_CH_SIDE_RIGHT (1ULL << AV_CHAN_SIDE_RIGHT )
> +#define AV_CH_TOP_CENTER (1ULL << AV_CHAN_TOP_CENTER )
> +#define
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On Tue, Dec 25, 2018 at 02:11:45PM +0100, Paul B Mahol wrote:
> On 12/25/18, Michael Niedermayer wrote:
> > On Mon, Dec 24, 2018 at 06:34:30PM +0100, Paul B Mahol wrote:
> > [...]
> >> -static const char *get_channel_name(int channel_id)
> >> +const char *av_channel_name(enum AVChannel channel_id)
> >> {
> >
> >> if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
> >> return NULL;
> >> +if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names))
> >> +return "?";
> >
> > this looks like a untended duplicate check
> >
> > [...]
> >
> >> +/**
> >> + * Check whether two channel layouts are semantically the same, i.e. the
> >> same
> >> + * channels are present on the same positions in both.
> >> + *
> >> + * If one of the channel layouts is AV_CHANNEL_ORDER_UNSPEC, while the
> >> other is
> >> + * not, they are considered to be unequal. If both are
> >> AV_CHANNEL_ORDER_UNSPEC,
> >> + * they are considered equal iff the channel counts are the same in
> >> both.
> >> + *
> >> + * @param chl input channel layout
> >> + * @param chl1 input channel layout
> >> + * @return 0 if chl and chl1 are equal, 1 if they are not equal. A
> >> negative
> >> + * AVERROR code if one or both are invalid.
> >> + */
> >> +int av_channel_layout_compare(const AVChannelLayout *chl, const
> >> AVChannelLayout *chl1);
> >
> > It could be usefull if this is a full compare function that allows
> > sorting/ordering
>
> Which kind of sorting?
it doesnt matter which sorting.
Having some allows some operations to be performed more efficient
than having none.
An example is merging 2 lists removing duplicates.
2 sorted lists can be merged in O(n) but if theres no way to sort
and just a equal / non equal check it requires O(n^2)
> That could be only added later when needed.
adding it later is fine but i think its better if the API
allows it.
This would not work with above as it requires "1 if they are not equal",
it could allow 1 or -1 (or just non zero) then it can be extended
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On 12/25/18, Michael Niedermayer wrote:
> On Mon, Dec 24, 2018 at 06:34:30PM +0100, Paul B Mahol wrote:
> [...]
>> -static const char *get_channel_name(int channel_id)
>> +const char *av_channel_name(enum AVChannel channel_id)
>> {
>
>> if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
>> return NULL;
>> +if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names))
>> +return "?";
>
> this looks like a untended duplicate check
>
> [...]
>
>> +/**
>> + * Check whether two channel layouts are semantically the same, i.e. the
>> same
>> + * channels are present on the same positions in both.
>> + *
>> + * If one of the channel layouts is AV_CHANNEL_ORDER_UNSPEC, while the
>> other is
>> + * not, they are considered to be unequal. If both are
>> AV_CHANNEL_ORDER_UNSPEC,
>> + * they are considered equal iff the channel counts are the same in
>> both.
>> + *
>> + * @param chl input channel layout
>> + * @param chl1 input channel layout
>> + * @return 0 if chl and chl1 are equal, 1 if they are not equal. A
>> negative
>> + * AVERROR code if one or both are invalid.
>> + */
>> +int av_channel_layout_compare(const AVChannelLayout *chl, const
>> AVChannelLayout *chl1);
>
> It could be usefull if this is a full compare function that allows
> sorting/ordering
Which kind of sorting? That could be only added later when needed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add a new channel layout API
On Mon, Dec 24, 2018 at 06:34:30PM +0100, Paul B Mahol wrote:
[...]
> -static const char *get_channel_name(int channel_id)
> +const char *av_channel_name(enum AVChannel channel_id)
> {
> if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
> return NULL;
> +if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names))
> +return "?";
this looks like a untended duplicate check
[...]
> +/**
> + * Check whether two channel layouts are semantically the same, i.e. the same
> + * channels are present on the same positions in both.
> + *
> + * If one of the channel layouts is AV_CHANNEL_ORDER_UNSPEC, while the other
> is
> + * not, they are considered to be unequal. If both are
> AV_CHANNEL_ORDER_UNSPEC,
> + * they are considered equal iff the channel counts are the same in both.
> + *
> + * @param chl input channel layout
> + * @param chl1 input channel layout
> + * @return 0 if chl and chl1 are equal, 1 if they are not equal. A negative
> + * AVERROR code if one or both are invalid.
> + */
> +int av_channel_layout_compare(const AVChannelLayout *chl, const
> AVChannelLayout *chl1);
It could be usefull if this is a full compare function that allows
sorting/ordering
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Add a new channel layout API
The new API is more extensible and allows for custom layouts.
More accurate information is exported, eg for decoders that do not
set a channel layout, lavc will not make one up for them.
Deprecate the old API working with just uint64_t bitmasks.
Original commit by Anton Khirnov .
Expanded and completed by Vittorio Giovara .
Adopted for FFmpeg by Paul B Mahol .
Signed-off-by: Anton Khirnov
Signed-off-by: Vittorio Giovara
Signed-off-by: Paul B Mahol
---
libavutil/channel_layout.c | 387 --
libavutil/channel_layout.h | 411 ++---
libavutil/version.h| 3 +
3 files changed, 711 insertions(+), 90 deletions(-)
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 3bd5ee29b7..a52f78bb9f 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -37,75 +37,89 @@ struct channel_name {
};
static const struct channel_name channel_names[] = {
- [0] = { "FL","front left"},
- [1] = { "FR","front right" },
- [2] = { "FC","front center" },
- [3] = { "LFE", "low frequency" },
- [4] = { "BL","back left" },
- [5] = { "BR","back right"},
- [6] = { "FLC", "front left-of-center" },
- [7] = { "FRC", "front right-of-center" },
- [8] = { "BC","back center" },
- [9] = { "SL","side left" },
-[10] = { "SR","side right"},
-[11] = { "TC","top center"},
-[12] = { "TFL", "top front left"},
-[13] = { "TFC", "top front center" },
-[14] = { "TFR", "top front right" },
-[15] = { "TBL", "top back left" },
-[16] = { "TBC", "top back center" },
-[17] = { "TBR", "top back right"},
-[29] = { "DL","downmix left" },
-[30] = { "DR","downmix right" },
-[31] = { "WL","wide left" },
-[32] = { "WR","wide right"},
-[33] = { "SDL", "surround direct left" },
-[34] = { "SDR", "surround direct right" },
-[35] = { "LFE2", "low frequency 2" },
+[AV_CHAN_FRONT_LEFT ] = { "FL" },
+[AV_CHAN_FRONT_RIGHT ] = { "FR" },
+[AV_CHAN_FRONT_CENTER] = { "FC" },
+[AV_CHAN_LOW_FREQUENCY ] = { "LFE" },
+[AV_CHAN_BACK_LEFT ] = { "BL" },
+[AV_CHAN_BACK_RIGHT ] = { "BR" },
+[AV_CHAN_FRONT_LEFT_OF_CENTER] = { "FLC" },
+[AV_CHAN_FRONT_RIGHT_OF_CENTER ] = { "FRC" },
+[AV_CHAN_BACK_CENTER ] = { "BC" },
+[AV_CHAN_SIDE_LEFT ] = { "SL" },
+[AV_CHAN_SIDE_RIGHT ] = { "SR" },
+[AV_CHAN_TOP_CENTER ] = { "TC" },
+[AV_CHAN_TOP_FRONT_LEFT ] = { "TFL" },
+[AV_CHAN_TOP_FRONT_CENTER] = { "TFC" },
+[AV_CHAN_TOP_FRONT_RIGHT ] = { "TFR" },
+[AV_CHAN_TOP_BACK_LEFT ] = { "TBL" },
+[AV_CHAN_TOP_BACK_CENTER ] = { "TBC" },
+[AV_CHAN_TOP_BACK_RIGHT ] = { "TBR" },
+[AV_CHAN_STEREO_LEFT ] = { "DL" },
+[AV_CHAN_STEREO_RIGHT] = { "DR" },
+[AV_CHAN_WIDE_LEFT ] = { "WL" },
+[AV_CHAN_WIDE_RIGHT ] = { "WR" },
+[AV_CHAN_SURROUND_DIRECT_LEFT] = { "SDL" },
+[AV_CHAN_SURROUND_DIRECT_RIGHT ] = { "SDR" },
+[AV_CHAN_LOW_FREQUENCY_2 ] = { "LFE2" },
+[AV_CHAN_SILENCE ] = { "PAD" },
};
-static const char *get_channel_name(int channel_id)
+const char *av_channel_name(enum AVChannel channel_id)
{
if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
return NULL;
+if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names))
+return "?";
return channel_names[channel_id].name;
}
+int av_channel_from_string(const char *str)
+{
+for (int i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
+if (channel_names[i].name && !strcmp(str, channel_names[i].name)) {
+return i;
+}
+}
+return AVERROR(EINVAL);
+}
+
static const struct {
const char *name;
-int nb_channels;
-uint64_t layout;
+AVChannelLayout layout;
} channel_layout_map[] = {
-{ "mono",1, AV_CH_LAYOUT_MONO },
-{ "stereo", 2, AV_CH_LAYOUT_STEREO },
-{ "2.1", 3, AV_CH_LAYOUT_2POINT1 },
-{ "3.0", 3, AV_CH_LAYOUT_SURROUND },
-{ "3.0(back)", 3, AV_CH_LAYOUT_2_1 },
-{ "4.0", 4, AV_CH_LAYOUT_4POINT0 },
-{ "quad",4, AV_CH_LAYOUT_QUAD },
-{ "quad(side)", 4, AV_CH_LAYOUT_2_2 },
-{ "3.1", 4, AV_CH_LAYOUT_3POINT1 },
-{ "5.0", 5, AV_CH_LAYOUT_5POINT0_BACK },
-{ "5.0(side)", 5, AV_CH_LAYOUT_5POINT0 },
-{
