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 <f...@google.com> wrote:
> 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
>>
>
From 75a2470f26ea0d4c5bf8482001ce7d32212ba06f Mon Sep 17 00:00:00 2001
From: Felicia Lim <f...@google.com>
Date: Mon, 30 Jul 2018 12:59:44 -0700
Subject: [PATCH] avcodec/libopusenc: Fix warning when encoding ambisonics with
channel mapping 2
Also adds checks on the number of channels.
---
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);
+ }
+ break;
case 255:
ret = libopus_check_max_channels(avctx, 254);
break;
--
2.18.0.597.ga71716f1ad-goog
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
