[flac-dev] about word size in bitreader/bitwriter

[lvqcl]

There are preprocessor definitions in bitreader.c and bitwriter.c:

     /* Things should be fastest when this matches the machine word size */
     /* WATCHOUT: if you change this you must also change the following #defines down to SWAP_BE_WORD_TO_HOST below to match */
     /* WATCHOUT: there are a few places where the code will not work unless uint32_t is >= 32 bits wide */
     #define FLAC__BYTES_PER_WORD 4
     #define FLAC__BITS_PER_WORD 32
     #define FLAC__WORD_ALL_ONES ((FLAC__uint32)0xffffffff)
     ...

and the following code in bitreader.c:

     #if FLAC__BYTES_PER_WORD == 4
         switch(br->crc16_align) {
             case  0: crc = FLAC__CRC16_UPDATE((unsigned)(word >> 24), crc);
             case  8: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 16) & 0xff), crc);
             case 16: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 8) & 0xff), crc);
             case 24: br->read_crc16 = FLAC__CRC16_UPDATE((unsigned)(word & 0xff), crc);
         }
     #elif FLAC__BYTES_PER_WORD == 8
         switch(br->crc16_align) {
             case  0: crc = FLAC__CRC16_UPDATE((unsigned)(word >> 56), crc);
             case  8: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 48) & 0xff), crc);
             case 16: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 40) & 0xff), crc);
             case 24: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 32) & 0xff), crc);
             case 32: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 24) & 0xff), crc);
             case 40: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 16) & 0xff), crc);
             case 48: crc = FLAC__CRC16_UPDATE((unsigned)((word >> 8) & 0xff), crc);
             case 56: br->read_crc16 = FLAC__CRC16_UPDATE((unsigned)(word & 0xff), crc);
         }
     #else
     ...


This implies that there was intention to set FLAC__BYTES_PER_WORD to 8 on 64-bit
architectures. However currently the code in bitreader.c/bitwriter.c doesn't care
much about 64-bit words: there are places like

     const FLAC__uint32 mask2 = FLAC__WORD_ALL_ONES >> (31-parameter);

and FLAC__bitwriter_write_raw_uint64() simply calls FLAC__bitwriter_write_raw_uint32() twice...


And there were three patches --

<http://git.xiph.org/?p=flac.git;a=commitdiff;h=66bd44bacc28cb154b5c9349cf3288da9ec74501>
<http://git.xiph.org/?p=flac.git;a=commitdiff;h=387b72731dbf79450050987b9b36bf70f286b098>
<http://git.xiph.org/?p=flac.git;a=commitdiff;h=d30fe60fc616d626df2336e91bc698677a732447>

that replaced bitness-agnostic brword/bwword and COUNT_ZERO_MSBS() with uint32_t and
FLAC__clz_uint32()  (which are apparently 32-bit only).


Does it make sense to restore the ability to use FLAC__BYTES_PER_WORD == 8?
(Honestly, I'm not sure that it ever worked previously... and I'm not sure that
this change can improve libFLAC performance).

Or is it better to change the comment that implies that it's possible?