[opus] [PATCH] Add Functions to Create Ambisonic Multistream Encoder
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
> 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).
> > 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
> Michael Graczyk
More information about the opus