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

Jean-Marc Valin jmvalin at jmvalin.ca
Wed May 25 03:22:39 UTC 2016


On 05/24/2016 10:33 PM, Michael Graczyk wrote:
> 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! 

The issue isn't errors in the code. If I took your previous patch and
attempted to compile it using the Makefile.unix that we provide, if
would not have worked since Makefile.unix doesn't define that flag.

> Either way I changed them to #ifdef everywhere.

Thanks. Your patch is now merged into the (increasingly inaccurately
named) exp_lbr_tune branch, where the current experimental work is
happening.

Cheers,

	Jean-Marc

> 
> 
> On Tue, May 24, 2016 at 8:26 AM, Jean-Marc Valin <jmvalin at jmvalin.ca
> <mailto:jmvalin at jmvalin.ca>> wrote:
> 
>     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>
>     > <mailto: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
>     >
> 
> 
> 
> 
> -- 
> 
> Thanks,
> Michael Graczyk


More information about the opus mailing list