Friendly ping. Please let me know if there are any changes I should make to
this patch?
Thanks!
Felicia
On Thu, Aug 9, 2018 at 6:41 AM Felicia Lim <f...@google.com> wrote:
> 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
>>>
>>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
