similar to: [PATCH] Add Functions to Create Ambisonic Multistream Encoder

Hi Michael, Is there any reason you can't just use the generic multi-stream API, i.e. opus_multistream_encoder_init() and give it the mapping you need? This is how surround was originally done (in 1.0) and only got changed when surround needed a more complex mapping and more data in the encoder. If it turns out you need this kind of thing too, then yes we would probably just want to extend

Jean-Marc, > Is there any reason you can't just use the generic multi-stream API, > i.e. opus_multistream_encoder_init() and give it the mapping you need? I would like the encoder to make decisions based on the fact that the audio is Ambisonics. For example, the bitrate allocation should not be the same for every channel. The original multistream API does not provide a way to pass in

Here is the modified patch. I added a flag to configure.ac which is set to 0 to disable ambisonics, and 1 to enable it. Right now the implementation simply creates a surround encoder with N uncoupled streams. Thanks, Michael Graczyk -------------- next part -------------- An HTML attachment was scrubbed... URL:

Alright, I've made the changes to the enum and flag usage. I'm don't see the difference with respect to "disabling blocks of code", since the compiler should completely elide the blocks that are conditioned on a constant 0. If there were compiler errors in the experimental code there could be a difference, but I'm hoping that is not the case on any platform! Either way I

Sounds good, I'll make those changes. Thanks for pointing out the make differences. Do you mind if I add an #ifndef, #define to the top of the file for the experiment flag? The code becomes significantly more nasty with #ifdefs everywhere and it would only get worse in subsequent patches. On May 23, 2016 21:28, "Jean-Marc Valin" <jmvalin at jmvalin.ca> wrote: > On

Hi Michael, Any particular reason you assign explicit values to your MappingType enum? If not, I'd rather not depend on the int values. Other than that, I think your patch is good to go. Cheers, Jean-Marc On 05/23/2016 01:27 PM, Michael Graczyk wrote: > Hi Jean-Marc, > > On Sat, May 21, 2016 at 1:58 PM, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: >> 1) I think

Does anyone have more thoughts on this patch? I would like to get this in so I can send more involved patches related to ambisonics. Also, do you guys prefer to do code review using pull requests on github? It seems that both are used for Opus. On Fri, May 6, 2016 at 4:46 PM, Michael Graczyk <mgraczyk at google.com> wrote: > Here is the modified patch. I added a flag to configure.ac

Hi Michael, Sorry, I ended up looking at an older version and got confused. The new one looks good and I only have a few nits: 1) I think "allocation mode" should be renamed to "mapping type" or something similar that to make it less confusing. 2) After discussing with Tim, the conclusion is that we should use mapping family 254 in the short term. We can switch to mapping

Since they correspond to mapping family values, I figured it would be wise to make them match so that the correspondence would be clear. If you would rather that correspondence not be explicit I will remove the explicit assignments. -------------- next part -------------- An HTML attachment was scrubbed... URL:

Hi Michael, On 05/24/2016 01:14 AM, Michael Graczyk wrote: > Thanks for pointing out the make differences. Do you mind if I add an > #ifndef, #define to the top of the file for the experiment flag? The > code becomes significantly more nasty with #ifdefs everywhere and it > would only get worse in subsequent patches. Unless it makes the code really bad, I'd rather use #ifdefs

On 05/05/2016 06:29 PM, Michael Graczyk wrote: > Alright, that is simpler. I can modify the patch to simply add > "mapping_family == 2" to the allowed values in the surround encoder > functions. Sounds reasonable. I would also suggest putting ambisonics behind a #define ENABLE_EXPERIMENTAL_AMBISONICS (or similar) until the spec is final to avoid accidental deployment of

On 05/23/2016 10:57 PM, Michael Graczyk wrote: > Since they correspond to mapping family values, I figured it would be > wise to make them match so that the correspondence would be clear. If > you would rather that correspondence not be explicit I will remove the > explicit assignments. I'd rather have them be different, since they're not really the same thing. The idea is

Change-Id: I16bd4cd990d8ad5888c9e30016218ac25242ecb5 --- src/opus_multistream_encoder.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/opus_multistream_encoder.c b/src/opus_multistream_encoder.c index 30cafe1..c07132f 100644 --- a/src/opus_multistream_encoder.c +++ b/src/opus_multistream_encoder.c @@ -747,7 +747,8 @@ static void ambisonics_rate_allocation( if

This patch adds a static bitrate allocation for ambisonics encoded with the multistream API. We allocate 4 bits to the mono channel (channel index 0) for every 3 bits allocated to other channels. The allocation is static because it does not depend on the input audio. This patch also forces the underlying encoders to use CELT only for ambisonics. -- Thanks, Michael Graczyk -------------- next

On May 20, 2016 18:32, "Jean-Marc Valin" <jmvalin at jmvalin.ca> wrote: > > Is it intended that this patch never sets ALLOCATION_MODE_AMBISONICS? > I'm having a hard time figuring out what it does in its current state. That was intended. I wanted to "reserve" the number 2 in this enum to make it clear that 2 would mean ambisonics in the future. > Also, I

Hi Jean-Marc, On Sat, May 21, 2016 at 1:58 PM, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > 1) I think "allocation mode" should be renamed to "mapping type" or > something similar that to make it less confusing. Done > 2) After discussing with Tim, the conclusion is that we should use > mapping family 254 in the short term. We can switch to mapping

On Sat, 28 May 2016 16:21:33 -0700, Michael Graczyk <mgraczyk at google.com> wrote : > Hi Marc, Hi Micheal. > On Sat, May 28, 2016 at 10:44 AM, Marc Lavallée <marc at hacklava.net> > wrote: > > I subscribed because your discussion on the IETF draft ("Ambisonics > > in an Ogg Opus Container") was mentioned on the sursound list. > > Thanks for

This patch adds a new option "channel-mapping" to opusenc which sets the channel mapping family used by the multistream encoder. Please let me know whether adding this option is worthwhile and whether the help string is okay. I tried to keep it short but accurate. The error message for an unimplemented channel mapping is "Error cannot create encoder: request not implemented".

Hi Opus list. I subscribed because your discussion on the IETF draft ("Ambisonics in an Ogg Opus Container") was mentioned on the sursound list. I tried Opus for ambisonics more than a year ago. It does works with uncoupled channels (I had to patch the encoder). I don't know what else could be done to optimize support for ambisonics, as I'm not a codec expert. So I think that

Tim, Would you mind giving me a more specific example of the sort of document that you think this should look like? I'd like to write up something that is somewhat final. On Mon, May 2, 2016 at 9:30 PM, Michael Graczyk <mgraczyk at google.com> wrote: > On Fri, Apr 29, 2016 at 4:32 PM, Timothy B. Terriberry > <tterribe at xiph.org> wrote: > > As a general point,