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

[Felicia Lim]

I've attached the patch with the updated description, please let me know if
I should make any other changes.

Thanks,
Felicia


On Mon, Jul 30, 2018 at 10:50 AM Felicia Lim <flim at google.com> wrote:

> 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
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avcodec-libopusenc-Fix-warning-when-encoding-ambison.patch
Type: text/x-patch
Size: 2031 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180808/d0dbdeed/attachment.bin>