[libav-devel] [PATCH] Add a new channel layout API

[Vittorio Giovara]

From: Anton Khirnov 

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.

Expanded and completed by Vittorio Giovara .
Signed-off-by: Vittorio Giovara 
---
Changes to documentation as suggested by wm4, completed the doxygen
documentation for a couple of functions.
Vittorio

 libavutil/channel_layout.c | 387 +++---
 libavutil/channel_layout.h | 408 ++---
 libavutil/version.h|   3 +
 3 files changed, 708 insertions(+), 90 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 41340ecdb6..285997446d 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -31,77 +31,90 @@
 #include "common.h"
 
 static const char * const 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];
 }
 
+int av_channel_from_string(const char *str)
+{
+int i;
+for (i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
+if (channel_names[i] && !strcmp(str, channel_names[i])) {
+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 },
-{ "stereo",  2,  AV_CH_LAYOUT_STEREO_DOWNMIX },
-{ "2.1", 3,  AV_CH_LAYOUT_2POINT1 },
-{ "3.0", 3,  AV_CH_LAYOUT_SURROUND },
-{ "3.0(back)",   3,  AV_CH_LAYOUT_2_1 },
-{ "3.1", 4,  AV_CH_LAYOUT_3POINT1 },
-{ "4.0", 4,  AV_CH_LAYOUT_4POINT0 },
-{ "quad",4,  AV_CH_LAYOUT_QUAD },
-{ "quad(side)",  4,  AV_CH_LAYOUT_2_2 },
-{ "4.1", 5,  AV_CH_LAYOUT_4POINT1 },
-{ "5.0", 5,  AV_CH_LAYOUT_5POINT0 },
-{ "5.0", 5,  AV_CH_LAYOUT_5POINT0_BACK },
-{ "5.1", 6,  AV_CH_LAYOUT_5POINT1 },
-{ "5.1", 6,  AV_CH_LAYOUT_5POINT1_BACK },
-{ "6.0", 6,  

Re: [libav-devel] [PATCH 01/25] Add a new channel layout API

[Vittorio Giovara]

On Thu, Jun 29, 2017 at 5:14 AM, wm4  wrote:
> On Wed, 28 Jun 2017 18:10:45 -0400
> Vittorio Giovara  wrote:
>>  /**
>> + * An AVChannelLayout holds information about the channel layout of audio 
>> data.
>> + *
>> + * A channel layout here is defined as a set of channels ordered in a 
>> specific
>> + * way (unless the channel order is AV_CHANNEL_ORDER_UNSPEC, in which case 
>> an
>> + * AVChannelLayout carries only the channel count).
>> + *
>> + * Unlike most structures in Libav, sizeof(AVChannelLayout) is a part of the
>> + * public ABI and may be used by the caller. E.g. it may be allocated on 
>> stack.
>
> You should specify how it has to be handled. You can:
> - default initialize it with {0} or by setting all used fields correctly
> - using a predefined layout as initializer (AV_CHANNEL_LAYOUT_STEREO
>   etc.)
> - initialize it with a constructor function
> - must be uninitialized with av_channel_layout_uninit() (at least in
>   some situations, which is weird)
> - copy via assigning is forbidden (probably?), av_channel_layout_copy()
>   must be used instead

Ok i'll add a verbatim of this description

>> + * No new fields may be added to it without a major version bump.
>
> I think it's still intended that you can add new fields to the union?
> So you will add new fields. You just won't change the size, or add
> new fields which are mandatory for already defined layout types.

ok I took it for granted, but mentioning it won't harm

>> + * An AVChannelLayout can be constructed using the convenience function
>> + * av_channel_layout_from_mask() / av_channel_layout_from_string(), or it 
>> can be
>> + * built manually by the caller.
>> + */
>> +typedef struct AVChannelLayout {
>> +/**
>> + * Channel order used in this layout.
>> + */
>
> ("Mandatory field.")

It is but it defaults to NATIVE order. I'll mention it anyway.

>> +enum AVChannelOrder order;
>> +
>> +/**
>> + * Number of channels in this layout. Mandatory field.
>> + */
>> +int nb_channels;
>> +
>> +/**
>> + * Details about which channels are present in this layout.
>> + * For AV_CHANNEL_ORDER_UNSPEC, this field is undefined and must not be
>> + * used.
>> + */
>> +union {
>> +/**
>> + * This member must be used for AV_CHANNEL_ORDER_NATIVE.
>> + * It is a bitmask, where the position of each set bit means that 
>> the
>> + * AVChannel with the corresponding value is present.
>> + *
>> + * I.e. when (mask & (1 << AV_CHAN_FOO)) is non-zero, then 
>> AV_CHAN_FOO
>> + * is present in the layout. Otherwise it is not present.
>> + *
>> + * @note when a channel layout using a bitmask is constructed or
>> + * modified manually (i.e.  not using any of the av_channel_layout_*
>> + * functions), the code doing it must ensure that the number of set 
>> bits
>> + * is equal to nb_channels.
>> + */
>> +uint64_t mask;
>> +/**
>> + * This member must be used when the channel order is
>> + * AV_CHANNEL_ORDER_CUSTOM. It is a nb_channels-sized array, with 
>> each
>> + * element signalling the presend of the AVChannel with the
>> + * corresponding value.
>> + *
>> + * I.e. when map[i] is equal to AV_CHAN_FOO, then AV_CH_FOO is the 
>> i-th
>> + * channel in the audio data.
>> + */
>> +enum AVChannel *map;
>
> Even if the channel map identifier is AV_CHAN_SILENCE? What does the
> data contain then, actual silence or undefined contents?

I suppose so, it will simply mean that the channel at position `i`
will be SILENCE.
I documented that channel as "empty", which is a little vague, do have
any suggestion?

>> +/**
>> + * @return a string describing a given channel.
>> + */
>> +const char *av_channel_name(enum AVChannel channel);
>
> What does it return for invalid channels?

it returns "?", I'll mention it in the docs

>> +
>> +/**
>> + * @return a channel described by the given string.
>> + */
>> +int av_channel_from_string(const char *name);
>
> Return what exactly? I guess AVChannel or negative error code. Could
> also say it's the inverse of av_channel_name().

okay

>> +
>> +/**
>> + * Initialize a native channel layout from a bitmask indicating which 
>> channels
>> + * are present.
>> + */
>> +void av_channel_layout_from_mask(AVChannelLayout *channel_layout, uint64_t 
>> mask);
>
> What does it do if *channel_layout is not set to all 0 bytes?

Unpredictable. I'll mention that the input layout should be properly
initialized.
(same for the questions below)

>> +
>> +/**
>> + * Free any allocated data in the channel layout and reset the channel
>> + * count to 0.
>> + */
>> +void av_channel_layout_uninit(AVChannelLayout *channel_layout);
>
> Can the user assume that for defined channel orders, which do not use
> allocated memory (like channel mask), this 

[libav-devel] [PATCH] lavr: switch to the new channel layout API

[Vittorio Giovara]

From: Anton Khirnov 

Set a whitelist of supported channel order.

Signed-off-by: Vittorio Giovara 
---
 libavresample/audio_mix.c| 148 ++--
 libavresample/audio_mix_matrix.c | 477 ++-
 libavresample/avresample.h   |  42 +++-
 libavresample/internal.h |  10 +-
 libavresample/options.c  |   8 +
 libavresample/tests/avresample.c |  26 +--
 libavresample/utils.c| 130 +++
 7 files changed, 507 insertions(+), 334 deletions(-)

diff --git a/libavresample/audio_mix.c b/libavresample/audio_mix.c
index 89ecc6ba71..36dff2b979 100644
--- a/libavresample/audio_mix.c
+++ b/libavresample/audio_mix.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include "libavutil/channel_layout.h"
 #include "libavutil/common.h"
 #include "libavutil/libm.h"
 #include "libavutil/samplefmt.h"
@@ -34,10 +35,8 @@ struct AudioMix {
 AVAudioResampleContext *avr;
 enum AVSampleFormat fmt;
 enum AVMixCoeffType coeff_type;
-uint64_t in_layout;
-uint64_t out_layout;
-int in_channels;
-int out_channels;
+AVChannelLayout in_layout;
+AVChannelLayout out_layout;
 
 int ptr_align;
 int samples_align;
@@ -331,8 +330,8 @@ static av_cold int mix_function_init(AudioMix *am)
 if (!am->mix) {
 av_log(am->avr, AV_LOG_ERROR, "audio_mix: NO FUNCTION FOUND: [fmt=%s] "
"[c=%s] [%d to %d]\n", av_get_sample_fmt_name(am->fmt),
-   coeff_type_names[am->coeff_type], am->in_channels,
-   am->out_channels);
+   coeff_type_names[am->coeff_type], am->in_layout.nb_channels,
+   am->out_layout.nb_channels);
 return AVERROR_PATCHWELCOME;
 }
 return 0;
@@ -358,38 +357,42 @@ AudioMix *ff_audio_mix_alloc(AVAudioResampleContext *avr)
 
 am->fmt  = avr->internal_sample_fmt;
 am->coeff_type   = avr->mix_coeff_type;
-am->in_layout= avr->in_channel_layout;
-am->out_layout   = avr->out_channel_layout;
-am->in_channels  = avr->in_channels;
-am->out_channels = avr->out_channels;
+
+ret = av_channel_layout_copy(>in_layout, >in_ch_layout);
+if (ret < 0)
+goto error;
+ret = av_channel_layout_copy(>out_layout, >out_ch_layout);
+if (ret < 0)
+goto error;
 
 /* build matrix if the user did not already set one */
 if (avr->mix_matrix) {
-ret = ff_audio_mix_set_matrix(am, avr->mix_matrix, avr->in_channels);
+ret = ff_audio_mix_set_matrix(am, avr->mix_matrix, 
avr->in_ch_layout.nb_channels);
 if (ret < 0)
 goto error;
 av_freep(>mix_matrix);
 } else {
-double *matrix_dbl = av_mallocz(avr->out_channels * avr->in_channels *
+double *matrix_dbl = av_mallocz(avr->out_ch_layout.nb_channels *
+avr->in_ch_layout.nb_channels *
 sizeof(*matrix_dbl));
 if (!matrix_dbl)
 goto error;
 
-ret = avresample_build_matrix(avr->in_channel_layout,
-  avr->out_channel_layout,
-  avr->center_mix_level,
-  avr->surround_mix_level,
-  avr->lfe_mix_level,
-  avr->normalize_mix_level,
-  matrix_dbl,
-  avr->in_channels,
-  avr->matrix_encoding);
+ret = avresample_build_matrix2(>in_ch_layout,
+   >out_ch_layout,
+   avr->center_mix_level,
+   avr->surround_mix_level,
+   avr->lfe_mix_level,
+   avr->normalize_mix_level,
+   matrix_dbl,
+   avr->in_ch_layout.nb_channels,
+   avr->matrix_encoding);
 if (ret < 0) {
 av_free(matrix_dbl);
 goto error;
 }
 
-ret = ff_audio_mix_set_matrix(am, matrix_dbl, avr->in_channels);
+ret = ff_audio_mix_set_matrix(am, matrix_dbl, 
avr->in_ch_layout.nb_channels);
 if (ret < 0) {
 av_log(avr, AV_LOG_ERROR, "error setting mix matrix\n");
 av_free(matrix_dbl);
@@ -402,7 +405,7 @@ AudioMix *ff_audio_mix_alloc(AVAudioResampleContext *avr)
 return am;
 
 error:
-av_free(am);
+ff_audio_mix_free();
 return NULL;
 }
 
@@ -422,11 +425,16 @@ void ff_audio_mix_free(AudioMix **am_p)
 memset(am->matrix_q15, 0, sizeof(am->matrix_q15));
 memset(am->matrix_flt, 0, sizeof(am->matrix_flt));
 
+av_channel_layout_uninit(>in_layout);
+av_channel_layout_uninit(>out_layout);
+
 av_freep(am_p);
 }
 
 int 

[libav-devel] [PATCH] lavr: Only allow pass-through of ambisonic channel layouts

[Vittorio Giovara]

Signed-off-by: Vittorio Giovara 
---
 libavresample/utils.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libavresample/utils.c b/libavresample/utils.c
index 5e4a9f3ac3..cadfd5474f 100644
--- a/libavresample/utils.c
+++ b/libavresample/utils.c
@@ -46,8 +46,10 @@ int avresample_open(AVAudioResampleContext *avr)
 
 /* whitelist allowed channel orders */
 if ( (avr->in_ch_layout.order != AV_CHANNEL_ORDER_NATIVE &&
+  avr->in_ch_layout.order != AV_CHANNEL_ORDER_AMBISONIC &&
   avr->in_ch_layout.order != AV_CHANNEL_ORDER_UNSPEC) ||
 (avr->out_ch_layout.order != AV_CHANNEL_ORDER_NATIVE &&
+ avr->out_ch_layout.order != AV_CHANNEL_ORDER_AMBISONIC &&
  avr->out_ch_layout.order != AV_CHANNEL_ORDER_UNSPEC)) {
 av_log(avr, AV_LOG_ERROR,
   "Input or ouput channel order is not supported.\n");
@@ -73,6 +75,14 @@ int avresample_open(AVAudioResampleContext *avr)
 av_channel_layout_default(>out_ch_layout, 
avr->out_ch_layout.nb_channels);
 }
 
+if (( avr->in_ch_layout.order == AV_CHANNEL_ORDER_AMBISONIC ||
+ avr->out_ch_layout.order == AV_CHANNEL_ORDER_AMBISONIC) &&
+av_channel_layout_compare(>in_ch_layout, >out_ch_layout)) {
+av_log(avr, AV_LOG_ERROR,
+   "Resampling to/from ambisonic channel layouts is not 
supported.\n");
+return AVERROR(ENOSYS);
+}
+
 /* set channel mixing parameters */
 #if FF_API_OLD_CHANNEL_LAYOUT
 if (avr->in_channel_layout) {
-- 
2.13.1

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 18/25] lavr: Only let pass-through ambisonic channel layouts

[Vittorio Giovara]

On Thu, Jun 29, 2017 at 5:23 AM, wm4  wrote:
> On Wed, 28 Jun 2017 18:11:02 -0400
> Vittorio Giovara  wrote:
>
>> Resampling or conversion to/from ambisonic audio are currently
>> unsupported features.
>>
>> Signed-off-by: Vittorio Giovara 
>> ---
>>  libavresample/utils.c | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/libavresample/utils.c b/libavresample/utils.c
>> index 15c827efbe..af2b9dbf2d 100644
>> --- a/libavresample/utils.c
>> +++ b/libavresample/utils.c
>> @@ -70,6 +70,14 @@ int avresample_open(AVAudioResampleContext *avr)
>>  av_channel_layout_default(>out_ch_layout, 
>> avr->out_ch_layout.nb_channels);
>>  }
>>
>> +if (( avr->in_ch_layout.order == AV_CHANNEL_ORDER_AMBISONIC ||
>> + avr->out_ch_layout.order == AV_CHANNEL_ORDER_AMBISONIC) &&
>> +av_channel_layout_compare(>in_ch_layout, >out_ch_layout)) 
>> {
>> +av_log(avr, AV_LOG_ERROR,
>> +   "Resampling to/from ambisonic channel layouts is not 
>> supported.\n");
>> +return AVERROR(ENOSYS);
>> +}
>> +
>>  /* set channel mixing parameters */
>>  #if FF_API_OLD_CHANNEL_LAYOUT
>>  if (avr->in_channel_layout) {
>
> Did it error out in all cases before this patch? It shouldn't use a
> blacklist for unknown channel orders, but a whitelist for supported
> ones.

In the past it used to support up any layout up to 62 channels, but it
never errored out.
I will change it to use a whitelist for only NATIVE and UNSPEC orders,
in the first lavr patch, and modify this one to only allow
passthrough.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 18/25] lavr: Only let pass-through ambisonic channel layouts

[Vittorio Giovara]

On Thu, Jun 29, 2017 at 3:22 AM, Diego Biurrun  wrote:
> On Wed, Jun 28, 2017 at 06:11:02PM -0400, Vittorio Giovara wrote:
>> Resampling or conversion to/from ambisonic audio are currently
>> unsupported features.
>
> Maybe you mean something like
>
>   lavr: Only allow pass-through of ambisonic channel layouts
>
> ? Your current commit title does not parse...

Yeah that is good. I tried to convey the fact that as long as you
don't change channel order, you're fine with using an ambisonic
channel layout for input and output.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 12/25] lavc: switch to the new channel layout API

[Vittorio Giovara]

On Thu, Jun 29, 2017 at 5:18 AM, wm4  wrote:
> On Wed, 28 Jun 2017 18:10:56 -0400
> Vittorio Giovara  wrote:
>
>> Since the request_channel_layout is used only by a handful of codecs,
>> move the option to codec private contexts.
>
> Not sure if that is justified...

I believe it is always good to move options out of the global state,
and this seems to be a perfect opportunity: beside adding the new API,
we are modifying the concept that a channel layout is not represented
(simply) by a bitmask, but by a series of features, such as map,
channel order, and count. The request_channel_layout is the last
holdout of that concept, so I think it makes sense to change and allow
for future expansion, for example an in-codec downmix from ambisonic
to normal stereo.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 08/25] libavfilter changes for the new channel layout API

[Vittorio Giovara]

On Thu, Jun 29, 2017 at 3:59 AM, Diego Biurrun  wrote:
> On Wed, Jun 28, 2017 at 06:10:52PM -0400, Vittorio Giovara wrote:
>> --- a/libavfilter/af_aformat.c
>> +++ b/libavfilter/af_aformat.c
>> @@ -94,6 +94,13 @@ static int get_sample_rate(const char *samplerate)
>> +static int get_channel_layout(const char *channel_layout)
>> +{
>> +AVChannelLayout ch_layout = {0};
>> --- a/libavfilter/af_channelmap.c
>> +++ b/libavfilter/af_channelmap.c
>> @@ -219,50 +223,59 @@ static av_cold int channelmap_init(AVFilterContext 
>> *ctx)
>>  if (s->channel_layout_str) {
>> -uint64_t fmt;
>> -if ((fmt = av_get_channel_layout(s->channel_layout_str)) == 0) {
>> +int ret;
>> +AVChannelLayout fmt = {0};
>
>> --- a/libavfilter/af_join.c
>> +++ b/libavfilter/af_join.c
>> @@ -285,12 +273,14 @@ static void guess_map_any(AVFilterContext *ctx, 
>> ChannelMap *ch,
>> -if ((inputs[i] & link->channel_layout) != link->channel_layout) {
>> -uint64_t unused = link->channel_layout & ~inputs[i];
>> +if ((inputs[i] & link->ch_layout.u.mask) != link->ch_layout.u.mask) 
>> {
>> +uint64_t unused = link->ch_layout.u.mask & ~inputs[i];
>> +AVChannelLayout layout = {0};
>
> spaces inside {}
>

applied locally

-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 20/25] lavf: Add non diegetic stream disposition flag

[Vittorio Giovara]

On Thu, Jun 29, 2017 at 4:02 AM, Diego Biurrun  wrote:
> On Wed, Jun 28, 2017 at 06:11:04PM -0400, Vittorio Giovara wrote:
>> Signed-off-by: Vittorio Giovara 
>> ---
>>  libavformat/avformat.h | 6 ++
>>  libavformat/dump.c | 2 ++
>>  2 files changed, 8 insertions(+)
>
> non-diegetic
>
>> --- a/libavformat/dump.c
>> +++ b/libavformat/dump.c
>> @@ -482,6 +482,8 @@ static void dump_stream_format(AVFormatContext *ic, int 
>> i,
>>  if (st->disposition & AV_DISPOSITION_CLEAN_EFFECTS)
>>  av_log(NULL, AV_LOG_INFO, " (clean effects)");
>> +if (st->disposition & AV_DISPOSITION_NON_DIEGETIC)
>> +av_log(NULL, AV_LOG_INFO, " (non diegetic)");
>
> same

applied locally

-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] Libav Sprint Pelhřimov

[Alexandra Hájková]

Hello everyone,

I would like to announce another Libav sprint in the wilds around
Pelhřimov, the date was set to 21-23 July, but the next weekend
(28-30) is also possible. The plans are:
* hacking
* cooking
* enjoying countryside
* evenutally others.
Please, reply here, if you're interested.

(The report about the last sprint:
http://sasshkas.blogspot.cz/2016/10/another-libav-sprint.html)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Diego Biurrun]

On Thu, Jun 29, 2017 at 10:32:16AM +0100, Mark Thompson wrote:
> On 29/06/17 08:31, Diego Biurrun wrote:
> > On Sun, Jun 18, 2017 at 07:08:02PM +0100, Mark Thompson wrote:
> >> --- /dev/null
> >> +++ b/libavutil/hwcontext_drm.h
> >> @@ -0,0 +1,145 @@
> >> +
> >> +#ifndef AVUTIL_HWCONTEXT_DRM_H
> >> +#define AVUTIL_HWCONTEXT_DRM_H
> >> +
> >> +#include "frame.h"
> > 
> > frame.h is not used,
> 
> AV_NUM_DATA_POINTERS
> 
> (Though there were thoughts that we could make this a new constant inside the 
> header, probably with the value 4.  Still undecided on that one.)
> 
> >  but stdint.h and sys/types.h are.
> 
> stddef.h and stdint.h are included by frame.h.  I don't see why sys/types.h 
> would be needed?

For size_t, but that's stddef.h, so you're right.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 07/12] lavfi: Add some common code for OpenCL filtering

[wm4]

On Thu, 29 Jun 2017 10:25:09 +0100
Mark Thompson  wrote:

> This does actually work already by magic :)
> 
> Both NV12 and P010 surfaces become UNORM R and RG planes, just with a 
> different size of sample underneath.  Use in OpenCL then sees them 
> identically as planes of single-precision floating point values.  
> (Technically it isn't quite right for P010 because the low bits don't 
> necessarily do the right thing, but it's close enough for practical purposes.)

You can assume the padding is 0 (or if it's not, that the source
actually has higher precision).

But scaling to full fixed point range instead of shifting is incorrect.

I'd love to see libplacebo to be used for this - but for now it doesn't
exist yet, and is OpenGL only.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Mark Thompson]

On 29/06/17 08:31, Diego Biurrun wrote:
> On Sun, Jun 18, 2017 at 07:08:02PM +0100, Mark Thompson wrote:
>> --- a/configure
>> +++ b/configure
>> @@ -190,6 +190,7 @@ External library support:
>>--enable-avisynth  video frameserver
>>--enable-avxsynth  Linux version of AviSynth
>>--enable-bzlib bzip2 compression [autodetect]
>> +  --enable-drm   DRM buffer sharing
>>--enable-frei0rvideo filtering plugins
>>--enable-gnutlscrypto
>>--enable-libbs2b   Bauer stereophonic-to-binaural DSP
>> @@ -1303,6 +1304,7 @@ EXTERNAL_LIBRARY_LIST="
>>  $EXTERNAL_LIBRARY_VERSION3_LIST
>>  avisynth
>>  avxsynth
>> +drm
>>  frei0r
>>  gnutls
>>  libbs2b
> 
> I think "libdrm" would be a better name for the component. AFAIU it is
> the actual name of the library.

Sure; will change.

>> @@ -4734,6 +4736,7 @@ done
>>  enabled avisynth  && require_header avisynth/avisynth_c.h
>>  enabled avxsynth  && require_header avxsynth/avxsynth_c.h
>>  enabled cuda  && require cuda cuda.h cuInit -lcuda
>> +enabled drm   && require_pkg_config libdrm libdrm xf86drm.h 
>> drmGetVersion
> 
> The first argument should be the build-system-internal name of the component,
> this will not work quite as expected. See what I wrote above.

Ok.

>> --- /dev/null
>> +++ b/libavutil/hwcontext_drm.c
>> @@ -0,0 +1,281 @@
>> --- /dev/null
>> +++ b/libavutil/hwcontext_drm.h
>> @@ -0,0 +1,145 @@
>> +
>> +#ifndef AVUTIL_HWCONTEXT_DRM_H
>> +#define AVUTIL_HWCONTEXT_DRM_H
>> +
>> +#include "frame.h"
> 
> frame.h is not used,

AV_NUM_DATA_POINTERS

(Though there were thoughts that we could make this a new constant inside the 
header, probably with the value 4.  Still undecided on that one.)

>  but stdint.h and sys/types.h are.

stddef.h and stdint.h are included by frame.h.  I don't see why sys/types.h 
would be needed?

> I think you also need to bump libavutil minor version.

Yes.  (Omitted for mergability while still seeking comments on the structure.)

Thanks,

- Mark

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 07/12] lavfi: Add some common code for OpenCL filtering

[Mark Thompson]

On 29/06/17 02:16, Jun Zhao wrote:
> On 2017/6/28 5:50, Mark Thompson wrote:
>> ---
>>  libavfilter/Makefile |   6 +
>>  libavfilter/opencl.c | 285 
>> +++
>>  libavfilter/opencl.h |  74 +++
>>  libavfilter/opencl/rgbyuv.cl | 117 ++
>>  libavfilter/opencl_source.h  |  24 
>>  tools/cl2c   |  20 +++
>>  6 files changed, 526 insertions(+)
> 
> I guess we can give a general Colour Space Conversions solution based on 
> OpenCL, now
> I can think some case can't support in this patch. :) e,g:
> 
> - 10bits <-> 8bits

This does actually work already by magic :)

Both NV12 and P010 surfaces become UNORM R and RG planes, just with a different 
size of sample underneath.  Use in OpenCL then sees them identically as planes 
of single-precision floating point values.  (Technically it isn't quite right 
for P010 because the low bits don't necessarily do the right thing, but it's 
close enough for practical purposes.)

> - YUV422 <-> 420P

Trickier because the layouts are genuinely different: 4:2:2 planar formats will 
be fine as-is (though requiring different OpenCL code), but there is no 
straightforward representation for single-plane interleaved YUYV so it's 
unclear what to do there.


(There are quite a lot of other holes in this as well - e.g. chroma siting and 
range aren't considered at all.)

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 18/25] lavr: Only let pass-through ambisonic channel layouts

[wm4]

On Wed, 28 Jun 2017 18:11:02 -0400
Vittorio Giovara  wrote:

> Resampling or conversion to/from ambisonic audio are currently
> unsupported features.
> 
> Signed-off-by: Vittorio Giovara 
> ---
>  libavresample/utils.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/libavresample/utils.c b/libavresample/utils.c
> index 15c827efbe..af2b9dbf2d 100644
> --- a/libavresample/utils.c
> +++ b/libavresample/utils.c
> @@ -70,6 +70,14 @@ int avresample_open(AVAudioResampleContext *avr)
>  av_channel_layout_default(>out_ch_layout, 
> avr->out_ch_layout.nb_channels);
>  }
>  
> +if (( avr->in_ch_layout.order == AV_CHANNEL_ORDER_AMBISONIC ||
> + avr->out_ch_layout.order == AV_CHANNEL_ORDER_AMBISONIC) &&
> +av_channel_layout_compare(>in_ch_layout, >out_ch_layout)) {
> +av_log(avr, AV_LOG_ERROR,
> +   "Resampling to/from ambisonic channel layouts is not 
> supported.\n");
> +return AVERROR(ENOSYS);
> +}
> +
>  /* set channel mixing parameters */
>  #if FF_API_OLD_CHANNEL_LAYOUT
>  if (avr->in_channel_layout) {

Did it error out in all cases before this patch? It shouldn't use a
blacklist for unknown channel orders, but a whitelist for supported
ones.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 17/25] channel_layout: Add Ambisonic components and channel order

[wm4]

On Wed, 28 Jun 2017 18:11:01 -0400
Vittorio Giovara  wrote:

> Signed-off-by: Vittorio Giovara 
> ---
>  libavutil/channel_layout.c | 86 
> --
>  libavutil/channel_layout.h | 33 ++
>  2 files changed, 116 insertions(+), 3 deletions(-)


>   * is equal to nb_channels.
> + *
> + * This member maybe be optionially used for 
> AV_CHANNEL_ORDER_AMBISONIC.
> + * It is a bitmask that indicates the channel layout of the last
> + * non-diegetic channels present in the stream.
>   */
>  uint64_t mask;

(Should say it's 0 for AV_CHANNEL_ORDER_AMBISONIC if unused.)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 12/25] lavc: switch to the new channel layout API

[wm4]

On Wed, 28 Jun 2017 18:10:56 -0400
Vittorio Giovara  wrote:

> Since the request_channel_layout is used only by a handful of codecs,
> move the option to codec private contexts.

Not sure if that is justified...
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 01/25] Add a new channel layout API

[wm4]

On Wed, 28 Jun 2017 18:10:45 -0400
Vittorio Giovara  wrote:

> From: Anton Khirnov 
> 
> 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.
> 
> Expanded and completed by Vittorio Giovara .
> Signed-off-by: Vittorio Giovara 
> ---

I guess this is essentially the final version, so I'm bikeshedding a
little bit harder. We'll probably have to live forever with this API,
anyway.

>  /**
> + * An AVChannelLayout holds information about the channel layout of audio 
> data.
> + *
> + * A channel layout here is defined as a set of channels ordered in a 
> specific
> + * way (unless the channel order is AV_CHANNEL_ORDER_UNSPEC, in which case an
> + * AVChannelLayout carries only the channel count).
> + *
> + * Unlike most structures in Libav, sizeof(AVChannelLayout) is a part of the
> + * public ABI and may be used by the caller. E.g. it may be allocated on 
> stack.

You should specify how it has to be handled. You can:
- default initialize it with {0} or by setting all used fields correctly
- using a predefined layout as initializer (AV_CHANNEL_LAYOUT_STEREO
  etc.)
- initialize it with a constructor function
- must be uninitialized with av_channel_layout_uninit() (at least in
  some situations, which is weird)
- copy via assigning is forbidden (probably?), av_channel_layout_copy()
  must be used instead

> + * No new fields may be added to it without a major version bump.

I think it's still intended that you can add new fields to the union?
So you will add new fields. You just won't change the size, or add
new fields which are mandatory for already defined layout types.

> + *
> + * An AVChannelLayout can be constructed using the convenience function
> + * av_channel_layout_from_mask() / av_channel_layout_from_string(), or it 
> can be
> + * built manually by the caller.
> + */
> +typedef struct AVChannelLayout {
> +/**
> + * Channel order used in this layout.
> + */

("Mandatory field.")

> +enum AVChannelOrder order;
> +
> +/**
> + * Number of channels in this layout. Mandatory field.
> + */
> +int nb_channels;
> +
> +/**
> + * Details about which channels are present in this layout.
> + * For AV_CHANNEL_ORDER_UNSPEC, this field is undefined and must not be
> + * used.
> + */
> +union {
> +/**
> + * This member must be used for AV_CHANNEL_ORDER_NATIVE.
> + * It is a bitmask, where the position of each set bit means that the
> + * AVChannel with the corresponding value is present.
> + *
> + * I.e. when (mask & (1 << AV_CHAN_FOO)) is non-zero, then 
> AV_CHAN_FOO
> + * is present in the layout. Otherwise it is not present.
> + *
> + * @note when a channel layout using a bitmask is constructed or
> + * modified manually (i.e.  not using any of the av_channel_layout_*
> + * functions), the code doing it must ensure that the number of set 
> bits
> + * is equal to nb_channels.
> + */
> +uint64_t mask;
> +/**
> + * This member must be used when the channel order is
> + * AV_CHANNEL_ORDER_CUSTOM. It is a nb_channels-sized array, with 
> each
> + * element signalling the presend of the AVChannel with the
> + * corresponding value.
> + *
> + * I.e. when map[i] is equal to AV_CHAN_FOO, then AV_CH_FOO is the 
> i-th
> + * channel in the audio data.
> + */
> +enum AVChannel *map;

Even if the channel map identifier is AV_CHAN_SILENCE? What does the
data contain then, actual silence or undefined contents?

> +} u;
> +} AVChannelLayout;
> +
> +#define AV_CHANNEL_LAYOUT_MONO \
> +{ .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 1,  .u = { .mask = 
> AV_CH_LAYOUT_MONO }}
> +#define AV_CHANNEL_LAYOUT_STEREO \
> +{ .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 2,  .u = { .mask = 
> AV_CH_LAYOUT_STEREO }}
> +#define AV_CHANNEL_LAYOUT_2POINT1 \
> +{ .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 3,  .u = { .mask = 
> AV_CH_LAYOUT_2POINT1 }}
> +#define AV_CHANNEL_LAYOUT_2_1 \
> +{ .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 3,  .u = { .mask = 
> AV_CH_LAYOUT_2_1 }}
> +#define AV_CHANNEL_LAYOUT_SURROUND \
> +{ .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 3,  .u = { .mask = 
> AV_CH_LAYOUT_SURROUND }}
> +#define AV_CHANNEL_LAYOUT_3POINT1 \
> +{ .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 4,  .u = { .mask = 
> AV_CH_LAYOUT_3POINT1 }}
> +#define AV_CHANNEL_LAYOUT_4POINT0 \
> +{ .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 4,  .u = { .mask = 
> AV_CH_LAYOUT_4POINT0 }}
> +#define AV_CHANNEL_LAYOUT_4POINT1 \
> +{ .order = 

Re: [libav-devel] [PATCH 04/12] hwcontext_opencl: QSV to OpenCL mapping for Intel Media SDK

[Diego Biurrun]

On Tue, Jun 27, 2017 at 10:50:46PM +0100, Mark Thompson wrote:
> --- a/configure
> +++ b/configure
> @@ -1720,6 +1720,7 @@ HAVE_LIST="
>  libc_msvcrt
>  MMAL_PARAMETER_VIDEO_MAX_NUM_CALLBACKS
>  opencl_beignet_vaapi
> +opencl_intel_media_vaapi

OK, I see that you add a second opencl + vaapi component. In that case
opencl_vaapi_ would sort the components naturally unlike having an
opencl_ prefix and a _vaapi suffix.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 03/12] hwcontext_opencl: VAAPI to OpenCL mapping for Intel i965+beignet

[Diego Biurrun]

On Tue, Jun 27, 2017 at 10:50:45PM +0100, Mark Thompson wrote:
> --- a/configure
> +++ b/configure
> @@ -1719,6 +1719,7 @@ HAVE_LIST="
>  dos_paths
>  libc_msvcrt
>  MMAL_PARAMETER_VIDEO_MAX_NUM_CALLBACKS
> +opencl_beignet_vaapi
>  sdl
>  section_data_rel_ro
>  threads
> @@ -4911,6 +4912,11 @@ enabled vaapi &&
>  enabled vaapi &&
>  check_lib vaapi_x11 "va/va.h va/va_x11.h" vaGetDisplay -lva -lva-x11 
> -lX11
>  
> +if enabled opencl && enabled vaapi ; then

if enabled_all opencl vaapi; then

> +check_type "CL/cl_intel.h" "clCreateImageFromFdINTEL_fn" &&
> +enable opencl_beignet_vaapi

I think plain "beignet" would be a better name for the component. The deps
don't need to be part of the name. "Beignet" is also what freedesktop
calls it.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 22/25] aac: Allow pass-through transcoding of ambisonic audio

[Diego Biurrun]

On Wed, Jun 28, 2017 at 06:11:06PM -0400, Vittorio Giovara wrote:
> The defacto mov standard mandates support for PCM and AAC: only the

de facto MOV

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 20/25] lavf: Add non diegetic stream disposition flag

[Diego Biurrun]

On Wed, Jun 28, 2017 at 06:11:04PM -0400, Vittorio Giovara wrote:
> Signed-off-by: Vittorio Giovara 
> ---
>  libavformat/avformat.h | 6 ++
>  libavformat/dump.c | 2 ++
>  2 files changed, 8 insertions(+)

non-diegetic

> --- a/libavformat/dump.c
> +++ b/libavformat/dump.c
> @@ -482,6 +482,8 @@ static void dump_stream_format(AVFormatContext *ic, int i,
>  if (st->disposition & AV_DISPOSITION_CLEAN_EFFECTS)
>  av_log(NULL, AV_LOG_INFO, " (clean effects)");
> +if (st->disposition & AV_DISPOSITION_NON_DIEGETIC)
> +av_log(NULL, AV_LOG_INFO, " (non diegetic)");

same

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 08/25] libavfilter changes for the new channel layout API

[Diego Biurrun]

On Wed, Jun 28, 2017 at 06:10:52PM -0400, Vittorio Giovara wrote:
> --- a/libavfilter/af_aformat.c
> +++ b/libavfilter/af_aformat.c
> @@ -94,6 +94,13 @@ static int get_sample_rate(const char *samplerate)
> +static int get_channel_layout(const char *channel_layout)
> +{
> +AVChannelLayout ch_layout = {0};
> --- a/libavfilter/af_channelmap.c
> +++ b/libavfilter/af_channelmap.c
> @@ -219,50 +223,59 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
>  if (s->channel_layout_str) {
> -uint64_t fmt;
> -if ((fmt = av_get_channel_layout(s->channel_layout_str)) == 0) {
> +int ret;
> +AVChannelLayout fmt = {0};

> --- a/libavfilter/af_join.c
> +++ b/libavfilter/af_join.c
> @@ -285,12 +273,14 @@ static void guess_map_any(AVFilterContext *ctx, 
> ChannelMap *ch,
> -if ((inputs[i] & link->channel_layout) != link->channel_layout) {
> -uint64_t unused = link->channel_layout & ~inputs[i];
> +if ((inputs[i] & link->ch_layout.u.mask) != link->ch_layout.u.mask) {
> +uint64_t unused = link->ch_layout.u.mask & ~inputs[i];
> +AVChannelLayout layout = {0};

spaces inside {}

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Diego Biurrun]

On Sun, Jun 18, 2017 at 07:08:02PM +0100, Mark Thompson wrote:
> --- a/configure
> +++ b/configure
> @@ -190,6 +190,7 @@ External library support:
>--enable-avisynth  video frameserver
>--enable-avxsynth  Linux version of AviSynth
>--enable-bzlib bzip2 compression [autodetect]
> +  --enable-drm   DRM buffer sharing
>--enable-frei0rvideo filtering plugins
>--enable-gnutlscrypto
>--enable-libbs2b   Bauer stereophonic-to-binaural DSP
> @@ -1303,6 +1304,7 @@ EXTERNAL_LIBRARY_LIST="
>  $EXTERNAL_LIBRARY_VERSION3_LIST
>  avisynth
>  avxsynth
> +drm
>  frei0r
>  gnutls
>  libbs2b

I think "libdrm" would be a better name for the component. AFAIU it is
the actual name of the library.

> @@ -4734,6 +4736,7 @@ done
>  enabled avisynth  && require_header avisynth/avisynth_c.h
>  enabled avxsynth  && require_header avxsynth/avxsynth_c.h
>  enabled cuda  && require cuda cuda.h cuInit -lcuda
> +enabled drm   && require_pkg_config libdrm libdrm xf86drm.h 
> drmGetVersion

The first argument should be the build-system-internal name of the component,
this will not work quite as expected. See what I wrote above.

> --- /dev/null
> +++ b/libavutil/hwcontext_drm.c
> @@ -0,0 +1,281 @@
> --- /dev/null
> +++ b/libavutil/hwcontext_drm.h
> @@ -0,0 +1,145 @@
> +
> +#ifndef AVUTIL_HWCONTEXT_DRM_H
> +#define AVUTIL_HWCONTEXT_DRM_H
> +
> +#include "frame.h"

frame.h is not used, but stdint.h and sys/types.h are.

I think you also need to bump libavutil minor version.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 18/25] lavr: Only let pass-through ambisonic channel layouts

[Diego Biurrun]

On Wed, Jun 28, 2017 at 06:11:02PM -0400, Vittorio Giovara wrote:
> Resampling or conversion to/from ambisonic audio are currently
> unsupported features.

Maybe you mean something like

  lavr: Only allow pass-through of ambisonic channel layouts

? Your current commit title does not parse...

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 24/25] fate: Add ambisonic tests

[Diego Biurrun]

On Wed, Jun 28, 2017 at 06:11:08PM -0400, Vittorio Giovara wrote:
> Signed-off-by: Vittorio Giovara 
> ---
>  tests/fate/mov.mak| 3 +++
>  tests/fate/opus.mak   | 5 +
>  tests/ref/fate/mov-ambisonic  | 1 +
>  tests/ref/fate/opus-ambisonic | 1 +
>  4 files changed, 10 insertions(+)
>  create mode 100644 tests/ref/fate/mov-ambisonic
>  create mode 100644 tests/ref/fate/opus-ambisonic

probably OK

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel