On Sat, Jul 28, 2018 at 10:46 AM Rostislav Pehlivanov <atomnu...@gmail.com>
wrote:
> On 27 July 2018 at 20:44, Felicia Lim <flim-at-google....@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@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
