Re: [FFmpeg-devel] [PATCH] avcodec/libopusenc: allow encoding channel mapping 2
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 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 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
>>> 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
Re: [FFmpeg-devel] [PATCH] [RFC] channel_layout: add support for ambisonics
On Thu, Nov 29, 2018 at 1:55 PM Marton Balint wrote:
>
>
> On Thu, 29 Nov 2018, Rostislav Pehlivanov wrote:
>
> > This is an RFC to add support for tagging channel layouts as ambisonics
> > in a backward-compatible way.
> > For now ambisonics up to third order are supported.
> > The functions have been updated to support and propagate the
> > AV_CH_LAYOUT_AMBISONICS flag.
> > This is messy but does not require a new API for layouts. Perhaps the
> > new proposed API might be a better solution, comments are welcome.
> > ---
> > doc/APIchanges | 4 ++
> > libavutil/channel_layout.c | 85 +-
> > libavutil/channel_layout.h | 19 -
> > libavutil/version.h| 4 +-
> > 4 files changed, 72 insertions(+), 40 deletions(-)
>
> Using separate channel_layout bits for each (up to 16) ambisonic channel
> seems a lot cleaner.
>
+1 for separate bits, e.g.
static const struct channel_name channel_names[] = {
...
+[36] = { "ACN_0", "ambisonics component 0" },
+[37] = { "ACN_1", "ambisonics component 1" },
as it would also enable signalling ambisonics with an additional
head-locked stereo track.
What do you think of maintaining ACN order throughout as {0, 1, 2, 3, 4, 5
...} instead of {1, 2, 0, 3, 4, 5 ...}?
Thanks,
Felicia
> Regards,
> Marton
> ___
> 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
[FFmpeg-devel] [PATCH] libopus: Add channel mapping 2 support in libopusdec
Hi all,
Here is another patch to decode Opus ambisonics files using channel mapping
2 [1], this time in libopusdec.c.
Please let me know if there are any concerns.
Thanks,
Felicia
[1] *https://trac.tools.ietf.org/html/draft-ietf-codec-ambisonics-02#section-3.1
<https://trac.tools.ietf.org/html/draft-ietf-codec-ambisonics-02#section-3.1>*
From 6607c6f8cff64771cdf34faa6e318271f5a48ac2 Mon Sep 17 00:00:00 2001
From: Felicia Lim
Date: Mon, 27 Mar 2017 16:21:20 -0700
Subject: [PATCH] libopus: Add channel mapping 2 support in libopusdec
Enables demuxing of Ambisonics content coded with channel mapping 2
---
libavcodec/libopusdec.c | 39 +--
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
index e6ca61a78f..9dab0fdf65 100644
--- a/libavcodec/libopusdec.c
+++ b/libavcodec/libopusdec.c
@@ -57,8 +57,6 @@ static av_cold int libopus_decode_init(AVCodecContext *avc)
avc->sample_rate= 48000;
avc->sample_fmt = avc->request_sample_fmt == AV_SAMPLE_FMT_FLT ?
AV_SAMPLE_FMT_FLT : AV_SAMPLE_FMT_S16;
-avc->channel_layout = avc->channels > 8 ? 0 :
- ff_vorbis_channel_layouts[avc->channels - 1];
if (avc->extradata_size >= OPUS_HEAD_SIZE) {
opus->pre_skip = AV_RL16(avc->extradata + 10);
@@ -82,14 +80,35 @@ static av_cold int libopus_decode_init(AVCodecContext *avc)
mapping= mapping_arr;
}
-if (avc->channels > 2 && avc->channels <= 8) {
-const uint8_t *vorbis_offset = ff_vorbis_channel_layout_offsets[avc->channels - 1];
-int ch;
-
-/* Remap channels from Vorbis order to ffmpeg order */
-for (ch = 0; ch < avc->channels; ch++)
-mapping_arr[ch] = mapping[vorbis_offset[ch]];
-mapping = mapping_arr;
+if (channel_map == 1) {
+avc->channel_layout = avc->channels > 8 ? 0 :
+ ff_vorbis_channel_layouts[avc->channels - 1];
+if (avc->channels > 2 && avc->channels <= 8) {
+const uint8_t *vorbis_offset = ff_vorbis_channel_layout_offsets[avc->channels - 1];
+int ch;
+
+/* Remap channels from Vorbis order to ffmpeg order */
+for (ch = 0; ch < avc->channels; ch++)
+mapping_arr[ch] = mapping[vorbis_offset[ch]];
+mapping = mapping_arr;
+}
+} else if (channel_map == 2) {
+int ambisonic_order = ff_sqrt(avc->channels) - 1;
+if (avc->channels != (ambisonic_order + 1) * (ambisonic_order + 1) &&
+avc->channels != (ambisonic_order + 1) * (ambisonic_order + 1) + 2) {
+av_log(avc, AV_LOG_ERROR,
+ "Channel mapping 2 is only specified for channel counts"
+ " which can be written as (n + 1)^2 or (n + 2)^2 + 2"
+ " for nonnegative integer n\n");
+return AVERROR_INVALIDDATA;
+}
+if (avc->channels > 227) {
+av_log(avc, AV_LOG_ERROR, "Too many channels\n");
+return AVERROR_INVALIDDATA;
+}
+avc->channel_layout = 0;
+} else {
+avc->channel_layout = 0;
}
opus->dec = opus_multistream_decoder_create(avc->sample_rate, avc->channels,
--
2.12.2.564.g063fe858b8-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libopus: Add channel mapping 2 support in libopusdec
Hi, Just wanted to follow up and check if there any changes I should add to this patch? Thanks for taking a look. Felicia On Mon, Mar 27, 2017 at 5:35 PM Felicia Lim wrote: > Hi all, > > Here is another patch to decode Opus ambisonics files using channel > mapping 2 [1], this time in libopusdec.c. > > Please let me know if there are any concerns. > > Thanks, > Felicia > > [1] > *https://trac.tools.ietf.org/html/draft-ietf-codec-ambisonics-02#section-3.1 > <https://trac.tools.ietf.org/html/draft-ietf-codec-ambisonics-02#section-3.1>* > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libopus: decode ambisonics with non-diegetic sources
Hi all,
The attached patch allows libavcodec/opus to decode ambisonics with
non-diegetic stereo stream in an ogg/opus container, as is being added in
this IETF standards draft [1].
Please let me know if there are any concerns.
Thanks,
Felicia
[1] https://tools.ietf.org/html/draft-ietf-codec-ambisonics-01#section-3.1
From 0798655323605d44d4f75e48fbfc940be0ba7423 Mon Sep 17 00:00:00 2001
From: Felicia
Date: Mon, 6 Feb 2017 15:49:36 -0800
Subject: [PATCH] libopus: decode ambisonics with non-diegetic sources
Channel mapping 2 additionally supports a non-diegetic stereo track
appended to the end of a full-order ambisonics signal, such that the
total channel count is either
(n + 1) ^ 2, or
(n + 1) ^ 2 + 2
where n is the ambisonics order
---
libavcodec/opus.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libavcodec/opus.c b/libavcodec/opus.c
index 1eeb92c5bd..07e70fd7a4 100644
--- a/libavcodec/opus.c
+++ b/libavcodec/opus.c
@@ -373,10 +373,12 @@ av_cold int ff_opus_parse_extradata(AVCodecContext *avctx,
channel_reorder = channel_reorder_vorbis;
} else if (map_type == 2) {
int ambisonic_order = ff_sqrt(channels) - 1;
-if (channels != (ambisonic_order + 1) * (ambisonic_order + 1)) {
+if (channels != ((ambisonic_order + 1) * (ambisonic_order + 1)) &&
+channels != ((ambisonic_order + 1) * (ambisonic_order + 1) + 2)) {
av_log(avctx, AV_LOG_ERROR,
"Channel mapping 2 is only specified for channel counts"
- " which can be written as (n + 1)^2 for nonnegative integer n\n");
+ " which can be written as (n + 1)^2 or (n + 1)^2 + 2"
+ " for nonnegative integer n\n");
return AVERROR_INVALIDDATA;
}
layout = 0;
--
2.11.0.483.g087da7b7c-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libopus: decode ambisonics with non-diegetic sources
Hi all, Just wanted to follow up this patch which allows decoding opus ambisonics with an additional stereo stream at the end. Please let me know if there are any changes I should make to it? Thanks, Felicia On Fri, Feb 10, 2017 at 10:42 AM Felicia Lim wrote: > Hi all, > > The attached patch allows libavcodec/opus to decode ambisonics with > non-diegetic stereo stream in an ogg/opus container, as is being added in > this IETF standards draft [1]. > > Please let me know if there are any concerns. > > Thanks, > Felicia > > [1] https://tools.ietf.org/html/draft-ietf-codec-ambisonics-01#section-3.1 > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/libopusenc: allow encoding channel mapping 2
---
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.345.g5c9ce644c3-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/libopusenc: allow encoding channel mapping 2
On Sat, Jul 28, 2018 at 10:46 AM Rostislav Pehlivanov
wrote:
> On 27 July 2018 at 20:44, Felicia Lim
> 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
Re: [FFmpeg-devel] [PATCH] avcodec/libopusenc: allow encoding channel mapping 2
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 wrote:
> On Sat, Jul 28, 2018 at 10:46 AM Rostislav Pehlivanov
> wrote:
>
>> On 27 July 2018 at 20:44, Felicia Lim
>> 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
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
