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

Jean-Marc Valin jmvalin at jmvalin.ca
Tue May 24 15:26:00 UTC 2016


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 directly
since we'll want to use it to disable blocks of code and mostly because
everything else already uses that (it would be confusing).

Thanks,

	Jean-Marc


> On May 23, 2016 21:28, "Jean-Marc Valin" <jmvalin at jmvalin.ca
> <mailto:jmvalin at jmvalin.ca>> wrote:
> 
>     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
> 


More information about the opus mailing list