similar to: [PATCH] multistream encoder doesn't accept OPUS_SET_EXPERT_FRAME_DURATION_REQUEST

When encoding using the Opus multistream API, is it possible for opus_multistream_encode() to return 0? For example, what happens if multiple streams are being encoded, each with DTX enabled and all streams emit DTX packets at the same time? What about the case of a single stream with DTX enabled? Thanks Kevin O'Connor -------------- next part -------------- An HTML attachment was

I use the Opus multistream encoder for both mono and stereo encodings and after updating from 1.1.3 to 1.3 I noticed the size of the produced Opus files had doubled for 1-channel encodings whereas switching to the standand Opus encoder gave me roughly the same sizes as before. In these tests, I'm encoding an 8kHz mono stream containing only speech with the following options set: frame length

This patch adds top level functions to create an ambisonic multistream encoder. The implementation currently just calls the analogous surround sound functions with channel mapping 255 to create an encoder that bundles uncoupled streams. Forthcoming patches will actually set channel bitrate and other configuration. My main concern is that adding additional

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

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

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

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 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

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

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, 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

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

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

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:

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

Hello, for your consideration. The following patch moves the channel_pos() function from within the #if !defined(DISABLE_FLOAT_API). This change is required when compiling with FIXED_POINT and DISABLE_FLOAT_API defined. #### ### diff --git a/src/opus_multistream_encoder.c b/src/opus_multistream_encoder.c index 3efab53..6f3eb53 100644 --- a/src/opus_multistream_encoder.c +++

Hi Drew, I've had some time to dig more deeply into your patch. Here's some more in-depth comments: 1) I note that your OpusMSEncoder struct in private.h adds a subframe_mem[] that's not in opus_multistream_encoder.c. I assume it's due to a merge problem (that field was removed some time ago), but can you confirm/fix the issue? 2) I noticed your patch adds many OPUS_EXPORT