[flac-dev] [PATCH 2/2] bitmath: Finish up optimizations
This patch adds support for other compilers and systems
including MSVC, Intel C compiler etc..
---
src/libFLAC/bitmath.c | 48 -
src/libFLAC/bitreader.c | 54 ++-
src/libFLAC/include/private/bitmath.h | 120 ++---
3 files changed, 116 insertions(+), 106 deletions(-)
diff --git a/src/libFLAC/bitmath.c b/src/libFLAC/bitmath.c
index 189977c..4fdde4b 100644
--- a/src/libFLAC/bitmath.c
+++ b/src/libFLAC/bitmath.c
@@ -36,54 +36,6 @@
#include "private/bitmath.h"
#include "FLAC/assert.h"
-/* An example of what FLAC__bitmath_ilog2() computes:
- *
- * ilog2( 0) = assertion failure
- * ilog2( 1) = 0
- * ilog2( 2) = 1
- * ilog2( 3) = 1
- * ilog2( 4) = 2
- * ilog2( 5) = 2
- * ilog2( 6) = 2
- * ilog2( 7) = 2
- * ilog2( 8) = 3
- * ilog2( 9) = 3
- * ilog2(10) = 3
- * ilog2(11) = 3
- * ilog2(12) = 3
- * ilog2(13) = 3
- * ilog2(14) = 3
- * ilog2(15) = 3
- * ilog2(16) = 4
- * ilog2(17) = 4
- * ilog2(18) = 4
- */
-
-#ifndef __GNUC__
-
-/* For GNUC, use static inline version in include/private/bitmath.h. */
-
-unsigned FLAC__bitmath_ilog2(FLAC__uint32 v)
-{
- unsigned l = 0;
-if (v == 0)
- return 0;
- while(v >>= 1)
- l++;
- return l;
-}
-
-unsigned FLAC__bitmath_ilog2_wide(FLAC__uint64 v)
-{
- unsigned l = 0;
-if (v == 0)
- return 0;
- while(v >>= 1)
- l++;
- return l;
-}
-#endif
-
/* An example of what FLAC__bitmath_silog2() computes:
*
* silog2(-10) = 5
diff --git a/src/libFLAC/bitreader.c b/src/libFLAC/bitreader.c
index dcd9e42..9e15db0 100644
--- a/src/libFLAC/bitreader.c
+++ b/src/libFLAC/bitreader.c
@@ -43,7 +43,7 @@
#include "share/endswap.h"
/* 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 COUNT_ZERO_MSBS below to match */
+/* WATCHOUT: if you change this you must also change the following #defines
down to FLAC__clz_uint32 below to match */
/* WATCHOUT: there are a few places where the code will not work unless
uint32_t is >= 32 bits wide */
/* also, some sections currently only have fast versions for 4 or 8
bytes per word */
#define FLAC__BYTES_PER_WORD 4 /* sizeof uint32_t */
@@ -56,27 +56,6 @@
#define SWAP_BE_WORD_TO_HOST(x) ENDSWAP_32(x)
#endif
-#if defined(__GNUC__)
-/* "int __builtin_clz (unsigned int x) If x is 0, the result is undefined" */
-static inline uint32_t
-COUNT_ZERO_MSBS (uint32_t word)
-{
- if (word == 0)
- return 32;
- return __builtin_clz (word);
-}
-#else
-/* counts the # of zero MSBs in a word */
-#define COUNT_ZERO_MSBS(word) ( \
- (word) > 0xff ? byte_to_unary_table[(word) >> 24] : \
- !(word) ? 32 : \
- (word) > 0x ? byte_to_unary_table[(word) >> 16] + 8 : \
- (word) > 0xff ? byte_to_unary_table[(word) >> 8] + 16 : \
- byte_to_unary_table[(word)] + 24 \
-)
-#endif
-
-
/*
* This should be at least twice as large as the largest number of words
* required to represent any 'number' (in any encoding) you are going to
@@ -93,25 +72,6 @@ COUNT_ZERO_MSBS (uint32_t word)
*/
static const unsigned FLAC__BITREADER_DEFAULT_CAPACITY = 65536u /
FLAC__BITS_PER_WORD; /* in words */
-static const unsigned char byte_to_unary_table[] = {
- 8, 7, 6, 6, 5, 5, 5, 5, 4, 4, 4, 4, 4, 4, 4, 4,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
- 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
- 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
- 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
-
/* adjust for compilers that can't understand using LLU suffix for uint64_t
literals */
#ifdef _MSC_VER
#define FLAC__U64L(x) x
@@ -679,7 +639,7 @@ FLAC__bool
FLAC__bitreader_read_unary_unsigned(FLAC__BitReader *br, unsigned *va
while(br->consumed_words < br->words) { /* if we've not
consumed up to a partial tail word... */
uint32_t b = br->buffer[br->consumed_words] <<
br->consumed_bits;
if(b) {
- i = COUNT_ZERO_MSBS(b);
+ i = FLAC__clz_uint32(b);
*val += i;
i++;
br
Re: [flac-dev] [PATCH] Add missing functions to VorbisComment class + a few other things
--- On Tue, 5/8/12, Erik de Castro Lopo wrote:
> Honestly, I really doubt this is a bug in valgrind :-). How
> were you testing
> this?
Well, I've read that there have been bugs in valgrind, were SSE optimized
versions of strlen() do guaranteed safe overreads of memory, but valgrind
wasn't aware the overreads were safe.
Anyway, it seems easy to reproduce for me, but I still may just be doing
something stupid:
[~]$ cat main.cc
#include
int main()
{
FLAC::Metadata::VorbisComment::Entry entry("ARTIST", "Someone cool and
terribly hip");
return 0;
}
[~]$ g++ -Wall -Wextra -O1 -g -lFLAC++ -o invalid_read main.cc
[~]$ valgrind ./invalid_read
==11150== Memcheck, a memory error detector
==11150== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==11150== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==11150== Command: ./invalid_read
==11150==
==11150== Invalid read of size 4
==11150==at 0x4030366:
FLAC::Metadata::VorbisComment::Entry::set_field_name(char const*)
(metadata.cpp:653)
==11150==by 0x7D5E35: (below main) (in /lib/libc-2.13.so)
==11150== Address 0x418802c is 4 bytes inside a block of size 7 alloc'd
==11150==at 0x4007D89: malloc (vg_replace_malloc.c:236)
==11150==by 0x83502F: strdup (in /lib/libc-2.13.so)
==11150==by 0x4030347:
FLAC::Metadata::VorbisComment::Entry::set_field_name(char const*)
(metadata.cpp:649)
==11150==by 0x7D5E35: (below main) (in /lib/libc-2.13.so)
==11150==
==11150==
==11150== HEAP SUMMARY:
==11150== in use at exit: 0 bytes in 0 blocks
==11150== total heap usage: 5 allocs, 5 frees, 119 bytes allocated
==11150==
==11150== All heap blocks were freed -- no leaks are possible
==11150==
==11150== For counts of detected and suppressed errors, rerun with: -v
==11150== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 21 from 8)
[~]$
Note, this is linked to current git (installed to /usr/local):
[~]$ ldd invalid_read | grep -i flac
libFLAC++.so.5 => /usr/local/lib/libFLAC++.so.5 (0x00358000)
libFLAC.so.8 => /usr/local/lib/libFLAC.so.8 (0x0011)
[~]$
The exact same program, linked to the distro's current stable release (1.2.1):
[~]$ g++ -Wall -Wextra -O1 -g -L/usr/lib -lFLAC++ -I/usr/include/ -o ok_read
-Xlinker -rpath -Xlinker /usr/lib main.cc
[~]$ ldd ok_read | grep -i flac
libFLAC++.so.6 => /usr/lib/libFLAC++.so.6 (0x00f89000)
libFLAC.so.8 => /usr/lib/libFLAC.so.8 (0x061d)
[~]$ valgrind ./ok_read
==11241== Memcheck, a memory error detector
==11241== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==11241== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==11241== Command: ./ok_read
==11241==
==11241==
==11241== HEAP SUMMARY:
==11241== in use at exit: 0 bytes in 0 blocks
==11241== total heap usage: 5 allocs, 5 frees, 119 bytes allocated
==11241==
==11241== All heap blocks were freed -- no leaks are possible
==11241==
==11241== For counts of detected and suppressed errors, rerun with: -v
==11241== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 21 from 8)
[~]$
I still have this creeping suspicion I'm missing something obvious...
Thanks for looking at and applying my patches the last few days by the way!
Bas
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] [PATCH] Add missing functions to VorbisComment class + a few other things
Bastiaan Timmer wrote: > Anyway, here's a patch that casts to FLAC__uint32, Applied. Thanks. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] [PATCH] Add missing functions to VorbisComment class + a few other things
Bastiaan Timmer wrote: > Attached is a patch that adds 5 missing Patch applied. Thanks. > Looking at the FLAC__metadata_object_cuesheet_* FLAC__metadata_ > object_picture_* functions, it looks like the corresponding FLAC++ > classes are already complete. Maybe some functions are missing from > CueSheet::Track. If nobody objects, I will take a look later this > week. Thanks. > Also, I've noticed that on my system, flac will not compile with > --enable-debug, because some functions that use 'PRId64' get compiled > in, but no included header defines 'PRId64'. Fixed, thanks. > Lastly, the reason I tried to compile with debug was some invalid read > sizes reported by valgrind when creating a VorbisComment::Entry. The > invalid read is reported in set_field_name() at line 653 ( > src/libFLAC++/metadata.cpp), where strlen() is used on the newly created > char *field_name_. I could not see anything wrong with the code though, > and strangely enough just calling printf on the field_name_ 1 line before > the strlen() removes all valgrind errors. So I'm not sure what's going on, > but it's probably a bug in valgrind, maybe somebody on this list knows? Honestly, I really doubt this is a bug in valgrind :-). How were you testing this? Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] [PATCH] Optionally, allow distros to use openssl for MD5 verification
Cristian RodrÃguez wrote: > flac and metaflac do not use openSSL, only libFLAC does. But flac and metaflac are GPL and link (possibly statically) to libFLAC. > Personally I don't see any technical reason to use a different library > other than politics and obscure potential license incompatibilities. A license incompatibility is a license incompatiility and you can't just cover your eyes and pretend it isn't there. For instance in Debian, there are numerous packages that have libreadline (GPL) support disabled because the package uses OpenSSL. Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
