[opus] [PATCH] Add Functions to Create Ambisonic Multistream Encoder

[Jean-Marc Valin]

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 family 2
when standardization is a bit more advanced.

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?).

Cheers,

	Jean-Marc

On 05/20/2016 07:36 PM, Michael Graczyk wrote:
> 
> On May 20, 2016 18:32, "Jean-Marc Valin" <jmvalin at jmvalin.ca
> <mailto: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 think the ambisonics function should be completely disabled
>> (hidden behind EXPERIMENTAL_AMBISONICS/--enable-experimental-ambisonics
>> flags) until there's actually an approved spec to avoid problems in the
>> future.
> I believe I did that already. It should be the case that without that
> flag, this patch is a noop. Did I miss something?
> 
>>As for the mapping family to use, I'm not yet sure whether it's
>> best to use 2 or 255 for now -- Tim, any opinion?
> Isn't 255 best reserved to mean "no mapping" as it does now?
>