opus - May 2016 - [PATCH] Add Functions to Create Ambisonic Multistream Encoder

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 "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 family 2
>> when standardization is a bit more advanced.
> Sounds good, done.
> 
>> 3) Unless it causes problems in the existing code, it seems like family
>> 254 should already set mapping_type=MAPPING_TYPE_AMBISONICS. From my
>> understanding, it would not change the code behaviour, but would make
it
>> clearer what the new type is for (or did I miss something?).
> Done, I think. The patch still doesn't change behavior, but it now
> sets the member OpusMSEncoder.mapping_type to MAPPING_TYPE_AMBISONICS.
>

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:
<http://lists.xiph.org/pipermail/opus/attachments/20160523/6781000b/attachment.html>

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 that we may have more than one family map to the same
type (e.g. currently both families 0 and 255 map to TYPE_NONE).

One last thing I just noticed, please don't rely on
ENABLE_EXPERIMENTAL_AMBISONICS actually being defined to zero when your
patch is disabled. Some people don't use the autotools build (e.g. they
use the plain Makefile or Visual Studio, or custom scripts), so your
patch would break their build. Instead, please rely on #ifdef's.

Cheers,

	Jean-Marc

>>> Jean-Marc Valin <jmvalin at jmvalin.ca> schrieb am 24.05.2016
um 03:31 in
Nachricht <5743AEDF.3070702 at jmvalin.ca>:
> 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.
Hi!

I think once you specify a binary format, the bits are important; if you only
look at the C source you shouldn't care about the actual bits when using
enums.

Ulrich
> 
> 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 "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
family 2
>>> when standardization is a bit more advanced.
>> Sounds good, done.
>> 
>>> 3) Unless it causes problems in the existing code, it seems like
family
>>> 254 should already set mapping_type=MAPPING_TYPE_AMBISONICS. From
my
>>> understanding, it would not change the code behaviour, but would
make it
>>> clearer what the new type is for (or did I miss something?).
>> Done, I think. The patch still doesn't change behavior, but it now
>> sets the member OpusMSEncoder.mapping_type to MAPPING_TYPE_AMBISONICS.
>> 
> _______________________________________________
> opus mailing list
> opus at xiph.org 
> http://lists.xiph.org/mailman/listinfo/opus