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

2018-01-19 Thread Vittorio Giovara 
On Sat, Jul 22, 2017 at 10:40 AM, Anton Khirnov  wrote:

> Quoting Vittorio Giovara (2017-06-29 00:10:50)
> > diff --git a/libavresample/utils.c b/libavresample/utils.c
> > index bab2153b4b..15c827efbe 100644
> > --- a/libavresample/utils.c
> > +++ b/libavresample/utils.c
> > @@ -18,6 +18,7 @@
> >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> >   */
> >
> > +#include "libavutil/channel_layout.h"
> >  #include "libavutil/common.h"
> >  #include "libavutil/dict.h"
> >  #include "libavutil/error.h"
> > @@ -35,6 +36,7 @@
> >
> >  int avresample_open(AVAudioResampleContext *avr)
> >  {
> > +int in_ch, out_ch;
> >  int ret;
> >
> >  if (avresample_is_open(avr)) {
> > @@ -42,24 +44,67 @@ int avresample_open(AVAudioResampleContext *avr)
> >  return AVERROR(EINVAL);
> >  }
> >
> > +if ( avr->in_ch_layout.order == AV_CHANNEL_ORDER_CUSTOM ||
> > +avr->out_ch_layout.order == AV_CHANNEL_ORDER_CUSTOM) {
> > +av_log(avr, AV_LOG_ERROR,
> > +  "Resampling a custom channel layout order is not
> supported.\n");
> > +   return AVERROR(ENOSYS);
>
> Why? I don't remember if I tested it properly back then, but IIRC the
> matrix building code should work custom orders just fine.
>
> > +}
> > +
> > +if (avr->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> > +if (avr->in_ch_layout.nb_channels > 63) {
> > +   av_log(avr, AV_LOG_ERROR,
> > +  "Unspecified channel layout order is supported only
> for up "
> > +  "to 63 channels (got %d).\n",
> avr->in_ch_layout.nb_channels);
> > +   return AVERROR(ENOSYS);
> > +}
> > +av_channel_layout_default(>in_ch_layout,
> avr->in_ch_layout.nb_channels);
> > +}
> > +if (avr->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> > +if (avr->out_ch_layout.nb_channels > 63) {
> > +   av_log(avr, AV_LOG_ERROR,
> > +  "Unspecified channel layout order is supported only
> for up "
> > +  "to 63 channels (got %d).\n", avr->out_ch_layout.nb_
> channels);
> > +   return AVERROR(ENOSYS);
> > +}
> > +av_channel_layout_default(>out_ch_layout,
> avr->out_ch_layout.nb_channels);
> > +}
>
> Why are those needed? Seems they are redundant given the other checks
> right below.
>

You are right on both points, the code is fine (and simpler) without these
checks.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

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

2017-07-22 Thread Anton Khirnov 
Quoting Vittorio Giovara (2017-06-29 00:10:50)
> diff --git a/libavresample/utils.c b/libavresample/utils.c
> index bab2153b4b..15c827efbe 100644
> --- a/libavresample/utils.c
> +++ b/libavresample/utils.c
> @@ -18,6 +18,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>   */
>  
> +#include "libavutil/channel_layout.h"
>  #include "libavutil/common.h"
>  #include "libavutil/dict.h"
>  #include "libavutil/error.h"
> @@ -35,6 +36,7 @@
>  
>  int avresample_open(AVAudioResampleContext *avr)
>  {
> +int in_ch, out_ch;
>  int ret;
>  
>  if (avresample_is_open(avr)) {
> @@ -42,24 +44,67 @@ int avresample_open(AVAudioResampleContext *avr)
>  return AVERROR(EINVAL);
>  }
>  
> +if ( avr->in_ch_layout.order == AV_CHANNEL_ORDER_CUSTOM ||
> +avr->out_ch_layout.order == AV_CHANNEL_ORDER_CUSTOM) {
> +av_log(avr, AV_LOG_ERROR,
> +  "Resampling a custom channel layout order is not 
> supported.\n");
> +   return AVERROR(ENOSYS);

Why? I don't remember if I tested it properly back then, but IIRC the
matrix building code should work custom orders just fine.

> +}
> +
> +if (avr->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> +if (avr->in_ch_layout.nb_channels > 63) {
> +   av_log(avr, AV_LOG_ERROR,
> +  "Unspecified channel layout order is supported only for up 
> "
> +  "to 63 channels (got %d).\n", 
> avr->in_ch_layout.nb_channels);
> +   return AVERROR(ENOSYS);
> +}
> +av_channel_layout_default(>in_ch_layout, 
> avr->in_ch_layout.nb_channels);
> +}
> +if (avr->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> +if (avr->out_ch_layout.nb_channels > 63) {
> +   av_log(avr, AV_LOG_ERROR,
> +  "Unspecified channel layout order is supported only for up 
> "
> +  "to 63 channels (got %d).\n", 
> avr->out_ch_layout.nb_channels);
> +   return AVERROR(ENOSYS);
> +}
> +av_channel_layout_default(>out_ch_layout, 
> avr->out_ch_layout.nb_channels);
> +}

Why are those needed? Seems they are redundant given the other checks
right below.


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

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

2017-06-28 Thread Vittorio Giovara 
From: Anton Khirnov 

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| 127 +++
 7 files changed, 504 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 ff_audio_mix(AudioMix *am, AudioData *src)
 {
+int