[FFmpeg-devel] [PATCH] avcodec/libopusenc: allow encoding channel mapping 2

[Felicia Lim]

On Sat, Jul 28, 2018 at 10:46 AM Rostislav Pehlivanov <atomnuker at gmail.com>
wrote:

> On 27 July 2018 at 20:44, Felicia Lim <flim-at-google.com at ffmpeg.org>
> wrote:
>
> > ---
> >  libavcodec/libopusenc.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
> > index 4ae81b0bb2..6b450ec317 100644
> > --- a/libavcodec/libopusenc.c
> > +++ b/libavcodec/libopusenc.c
> > @@ -27,6 +27,7 @@
> >  #include "bytestream.h"
> >  #include "internal.h"
> >  #include "libopus.h"
> > +#include "mathops.h"
> >  #include "vorbis.h"
> >  #include "audio_frame_queue.h"
> >
> > @@ -200,6 +201,21 @@ static int
> libopus_check_vorbis_layout(AVCodecContext
> > *avctx, int mapping_family
> >      return 0;
> >  }
> >
> > +static int libopus_check_ambisonics_channels(AVCodecContext *avctx) {
> > +    int channels = avctx->channels;
> > +    int ambisonic_order = ff_sqrt(channels) - 1;
> > +    if (channels != ((ambisonic_order + 1) * (ambisonic_order + 1)) &&
> > +        channels != ((ambisonic_order + 1) * (ambisonic_order + 1) +
> 2)) {
> > +        av_log(avctx, AV_LOG_ERROR,
> > +               "Ambisonics coding is only specified for channel counts"
> > +               " which can be written as (n + 1)^2 or (n + 1)^2 + 2"
> > +               " for nonnegative integer n\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  static int libopus_validate_layout_and_get_channel_map(
> >          AVCodecContext *avctx,
> >          int mapping_family,
> > @@ -231,6 +247,12 @@ static int libopus_validate_layout_and_
> > get_channel_map(
> >              channel_map =
> ff_vorbis_channel_layout_offsets[avctx->channels
> > - 1];
> >          }
> >          break;
> > +    case 2:
> > +        ret = libopus_check_max_channels(avctx, 227);
> > +        if (ret == 0) {
> > +            ret = libopus_check_ambisonics_channels(avctx);
> > +        }
> >
>
> No brackets needed, also if (ret) looks better imo.
>

Thanks for reviewing. Would this change would break with the style
elsewhere in this function? I'm happy to change if you still think I should
do it.

I also just realized the patch description should be "Remove warning when
trying to encode channel mapping 2": it already encodes even without this
patch. Will send an update after the above comment is resolved.


> Does libopus understand channel mapping 2 as ambisonics? What about the
> libopus_generic_encoder_() API? Doesn't that need to be used to encode
> ambisonics, like the patch by Drew did?
>

Yes, libopus also understands channel mapping 2 as ambisonics by default
now.

Ch map 2 uses the normal opus_multistream_encode(), so no further changes
are needed. On the other hand, ch map 3 uses the new
opus_projection_encode(), hence Drew's introduction of
libopus_generic_encoder_() to handle the switch as needed.


_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>