Re: multiple_of macro seems wrong

Hi Robert!

Yes, the macro is plain wrong. I've changed it:

-#define multiple_of(CHUNK, COUNT) ((COUNT) + ((COUNT) % (CHUNK)))
+#define multiple_of(CHUNK, COUNT) (\
+  ( (COUNT) < 1 || (CHUNK) < 1 || (COUNT) % (CHUNK) == 0 ) \
+  ? (COUNT) \
+  : ((COUNT) + (CHUNK) - (COUNT) % (CHUNK)) \
+  )

and you are right, a multiple of 4 is enough too.


Ciao Robert


Am 20.09.2017, 11:45 Uhr, schrieb Robert Kausch <[email protected]>:

> Hi all,
>
> Looking at the new LAME 3.100 release candidate I found the multiple_of  
> macro defined in machine.h to be wrong.
>
> #define multiple_of(CHUNK, COUNT) ((COUNT) + ((COUNT) % (CHUNK)))
>
> It seems like it is meant to round up the value of COUNT to the next  
> multiple of CHUNK, but it doesn't. Filling the values 16 and 5 like in  
> the definition of ABButter in gain_analysis.c gives 5 + (5 % 16) = 10  
> instead of 16.
>
> A correct definition of the macro would be:
>
> #define multiple_of(CHUNK, COUNT) ((COUNT) + (CHUNK) - 1 - ((COUNT) +  
> (CHUNK) - 1) % (CHUNK))
>
> or:
>
> #define multiple_of(CHUNK, COUNT) ((((COUNT) + (CHUNK) - 1) / (CHUNK)) *  
> (CHUNK))
>
> (Inspired by the answers to this question on StackOverflow:  
> https://stackoverflow.com/questions/3407012/)
>
> Also, I think using a multiple of 4 should be enough for ABYule and  
> ABButter. You want 16 byte alignment for vectorization and float is 4  
> bytes, so 4 * 4 = 16 should be fine.
>
> Cheers,
> Robert
> ----
> Robert Kausch
> [email protected]

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot