More or less detailed explanation of this patch.
1. The first parameter of _BitScanReverse() and _BitScanReverse64() is a pointer to unsigned long (4-byte int). However _BitScanReverse64() is called with a pointer to FLAC__uint64 (8-byte int). IMHO it's a bug and this patch changes the type of idx variable inside FLAC__bitmath_ilog2_wide() from FLAC__uint64 to unsigned long. The type of idx variable inside FLAC__bitmath_ilog2() was also changed (from FLAC__uint32 to unsigned long), for symmetry. 2. FLAC__clz_uint32() returns the result of BitScanReverse() XOR 31. FLAC__bitmath_ilog2() returns 31-FLAC__clz_uint32() == 31-(BSR^31). As 0<=BSR<=31 so 31-(BSR^31) == BSR. But I don't think that compilers are so smart that can optimize this expression. So it is better to change FLAC__bitmath_ilog2() so it simply calls BitScanReverse() when possible. 3. FLAC__bitmath_ilog2_wide() cannot be compiled with MSVC: it has 'if (v == 0)...' statement followed by a variable (either idx or DEBRUIJN_IDX64) definition. But all calls of this function have FLAC__ASSERT statements that ensure that its argument is greater than 0. So this 'if()' can be removed. 4. Am I right that FLAC__bitmath_ilog2_wide() and FLAC__bitmath_ilog2() return the same value if their argument is less than 2^32? Then FLAC__bitmath_ilog2_wide() works correctly only when compiled with GCC: it should return idx, not idx^63U. Also the result of de Bruijn sequences is off by 1.
bitmath.patch
Description: Binary data
_______________________________________________ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
