Re: [flac-dev] [Flac-dev] Git branch with compiling fixes for win32
El 03/05/12 12:19, Miroslav Lichvar escribió: > Hi Josh, > > nice to see you here again. > > On Wed, Apr 25, 2012 at 04:26:05PM -0700, Josh Coalson wrote: >> (Jumping in again, maybe at the wrong point since this doesn't seem >> to involve encoding, but here goes.) >> >> Miroslav's patches have always been high-quality for sure. But >> regardless of submitter, any patch that affects encoding must be >> reviewed very carefully, preferably by several other people and >> definitely me. If there were ever a libFLAC release that had a bug >> and was not always lossless, that would be very damaging to the >> format. > > The bitreader patch touches only the rice decoding code which I > believe is very well covered by the test suite and any bugs would be > quickly seen. Also, it has also been included in the Fedora packages > for several years, no bug reports about MD5 mismatch were received > yet :). > > It makes the C function faster than the corresponding asm routine, so > if it's included I'd suggest to just drop the asm function to not keep > around more asm code than is necessary. > > I'm not sure if anyone is planning to port the asm code to x86_64, I > think that it will be quite a lot of work, perhaps it would be a good > time to reconsider using inline assembly instead of nasm to minimize > the amount of asm code? It would be useful to know how much are the > individual asm routines actually faster, it has been a long time > since I played with it. > Hi: Both Erick and I did already submitted patches to the tree that do just exactly what your flac-1.2.1-bitreader.patch intended.. please checkout current GIT tree. Cheers! ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] [Flac-dev] Git branch with compiling fixes for win32
El 03/05/12 12:19, Miroslav Lichvar escribió: > It makes the C function faster than the corresponding asm routine, so > if it's included I'd suggest to just drop the asm function to not keep > around more asm code than is necessary. With current compilers it is very likely that those routines are already superflous. ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] [Flac-dev] Git branch with compiling fixes for win32
On Fri, May 04, 2012 at 11:13:05AM -0400, Cristian Rodríguez wrote: > Both Erick and I did already submitted patches to the tree that do just > exactly what your flac-1.2.1-bitreader.patch intended.. please checkout > current GIT tree. The most interesting part of the patch is the rewrite of the FLAC__bitreader_read_rice_signed_block function, which in the git repo seems to have only couple lines changed since 1.2.1. BTW, how much faster is the code with the clz builtin? If the architecture doesn't have the instruction, will be the gcc code as fast as the original code? -- Miroslav Lichvar ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] [Flac-dev] Git branch with compiling fixes for win32
On Fri, May 04, 2012 at 05:53:23PM +0200, Miroslav Lichvar wrote: > The most interesting part of the patch is the rewrite of the > FLAC__bitreader_read_rice_signed_block function, which in the git repo > seems to have only couple lines changed since 1.2.1. Here is that part of the patch rebased against current git. In a quick test it gives a 10% speedup in decoding. -- Miroslav Lichvar >From 9d0f5cc0c75f211961f4654b83dfdd15832cc46c Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Fri, 4 May 2012 17:59:17 +0200 Subject: [PATCH] Optimize FLAC__bitreader_read_rice_signed --- src/libFLAC/bitreader.c | 445 +++ 1 files changed, 105 insertions(+), 340 deletions(-) diff --git a/src/libFLAC/bitreader.c b/src/libFLAC/bitreader.c index ae515a0..7ae086d 100644 --- a/src/libFLAC/bitreader.c +++ b/src/libFLAC/bitreader.c @@ -755,379 +755,144 @@ FLAC__bool FLAC__bitreader_read_rice_signed(FLAC__BitReader *br, int *val, unsig } /* this is by far the most heavily used reader call. it ain't pretty but it's fast */ -/* a lot of the logic is copied, then adapted, from FLAC__bitreader_read_unary_unsigned() and FLAC__bitreader_read_raw_uint32() */ FLAC__bool FLAC__bitreader_read_rice_signed_block(FLAC__BitReader *br, int vals[], unsigned nvals, unsigned parameter) -/* OPT: possibly faster version for use with MSVC */ -#ifdef _MSC_VER { - unsigned i; - unsigned uval = 0; - unsigned bits; /* the # of binary LSBs left to read to finish a rice codeword */ - /* try and get br->consumed_words and br->consumed_bits into register; * must remember to flush them back to *br before calling other -* bitwriter functions that use them, and before returning */ - register unsigned cwords; - register unsigned cbits; +* bitreader functions that use them, and before returning */ + unsigned cwords, words, lsbs, msbs, x, y; + unsigned ucbits; /* keep track of the number of unconsumed bits in word */ + uint32_t b; + int *val, *end; FLAC__ASSERT(0 != br); FLAC__ASSERT(0 != br->buffer); /* WATCHOUT: code does not work with <32bit words; we can make things much faster with this assertion */ FLAC__ASSERT(FLAC__BITS_PER_WORD >= 32); FLAC__ASSERT(parameter < 32); - /* the above two asserts also guarantee that the binary part never straddles more that 2 words, so we don't have to loop to read it */ - - if(nvals == 0) - return true; - - cbits = br->consumed_bits; - cwords = br->consumed_words; + /* the above two asserts also guarantee that the binary part never straddles more than 2 words, so we don't have to loop to read it */ - while(1) { + val = vals; + end = vals + nvals; - /* read unary part */ - while(1) { - while(cwords < br->words) { /* if we've not consumed up to a partial tail word... */ - uint32_t b = br->buffer[cwords] << cbits; - if(b) { -#if 0 /* slower, probably due to bad register allocation... */ && defined FLAC__CPU_IA32 && !defined FLAC__NO_ASM && FLAC__BITS_PER_WORD == 32 - __asm { - bsr eax, b - not eax - and eax, 31 - mov i, eax - } -#else - i = COUNT_ZERO_MSBS(b); -#endif - uval += i; - bits = parameter; - i++; - cbits += i; - if(cbits == FLAC__BITS_PER_WORD) { - crc16_update_word_(br, br->buffer[cwords]); - cwords++; - cbits = 0; - } - goto break1; - } - else { - uval += FLAC__BITS_PER_WORD - cbits; - crc16_update_word_(br, br->buffer[cwords]); - cwords++; - cbits = 0; - /* didn't find stop bit yet, have to keep going... */ - } - } - /* at this point we've eaten up all the whole words; have to try -* reading through any tail bytes before calling the read callback. -* this is a repeat of the above logic
Re: [flac-dev] [Flac-dev] Git branch with compiling fixes for win32
El 04/05/12 11:53, Miroslav Lichvar escribió: > On Fri, May 04, 2012 at 11:13:05AM -0400, Cristian Rodríguez wrote: >> Both Erick and I did already submitted patches to the tree that do just >> exactly what your flac-1.2.1-bitreader.patch intended.. please checkout >> current GIT tree. > > The most interesting part of the patch is the rewrite of the > FLAC__bitreader_read_rice_signed_block function, which in the git repo > seems to have only couple lines changed since 1.2.1. Ah ok, I will check it out ! > > BTW, how much faster is the code with the clz builtin? If the > architecture doesn't have the instruction, will be the gcc code as > fast as the original code? I do not have access to a host that does not have either bsr, clz, lzcnt, cntlz or ctlz. if you do, plz share your results ;) I have no idea what code will be generated in exotic archs, in x86 it translates to bsr ^ 31U and in the others supported to a single instruction. ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] [Flac-dev] Git branch with compiling fixes for win32
El 04/05/12 12:09, Miroslav Lichvar escribió: > On Fri, May 04, 2012 at 05:53:23PM +0200, Miroslav Lichvar wrote: >> The most interesting part of the patch is the rewrite of the >> FLAC__bitreader_read_rice_signed_block function, which in the git repo >> seems to have only couple lines changed since 1.2.1. > > Here is that part of the patch rebased against current git. In a quick > test it gives a 10% speedup in decoding. > Looking at the openSUSE trees, we carry this patch since Aug 2011 and have recieved no reports on it being broken, also it works fine in my machine ;) ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
[flac-dev] [PATCH] Add missing functions to SeekTable class
The attached patch adds the missing FLAC__metadata_object_seektable_*() functions from FLAC's metadata object methods (FLAC/metadata.h) to FLAC++'s SeekTable class. Of the 11 functions in the C API, only 4 are currently in the C++ API, this patch adds the missing 7. If this patch is ok, VorbisComment will be next. A quick look tells me only 5 out of 13 FLAC__metadata_object_vorbiscomment*() functions from FLAC/metadata.h exist in FLAC++'s VorbisComment class. Bas Timmerdiff --git a/include/FLAC++/metadata.h b/include/FLAC++/metadata.h index c23af91..94eb6fc 100644 --- a/include/FLAC++/metadata.h +++ b/include/FLAC++/metadata.h @@ -502,6 +502,9 @@ namespace FLAC { unsigned get_num_points() const; ::FLAC__StreamMetadata_SeekPoint get_point(unsigned index) const; + //! See FLAC__metadata_object_seektable_resize_points() + bool resize_points(unsigned new_num_points); + //! See FLAC__metadata_object_seektable_set_point() void set_point(unsigned index, const ::FLAC__StreamMetadata_SeekPoint &point); @@ -513,6 +516,24 @@ namespace FLAC { //! See FLAC__metadata_object_seektable_is_legal() bool is_legal() const; + + //! See FLAC__metadata_object_seektable_template_append_placeholders() + bool template_append_placeholders(unsigned num); + + //! See FLAC__metadata_object_seektable_template_append_point() + bool template_append_point(FLAC__uint64 sample_number); + + //! See FLAC__metadata_object_seektable_template_append_points() + bool template_append_points(FLAC__uint64 sample_numbers[], unsigned num); + + //! See FLAC__metadata_object_seektable_template_append_spaced_points() + bool template_append_spaced_points(unsigned num, FLAC__uint64 total_samples); + + //! See FLAC__metadata_object_seektable_template_append_spaced_points_by_samples() + bool template_append_spaced_points_by_samples(unsigned samples, FLAC__uint64 total_samples); + + //! See FLAC__metadata_object_seektable_template_sort() + bool template_sort(bool compact); }; /** VORBIS_COMMENT metadata block. diff --git a/src/libFLAC++/metadata.cpp b/src/libFLAC++/metadata.cpp index 3c9f657..0870d02 100644 --- a/src/libFLAC++/metadata.cpp +++ b/src/libFLAC++/metadata.cpp @@ -438,6 +438,12 @@ namespace FLAC { return object_->data.seek_table.points[index]; } + bool SeekTable::resize_points(unsigned new_num_points) + { + FLAC__ASSERT(is_valid()); + return (bool)::FLAC__metadata_object_seektable_resize_points(object_, new_num_points); + } + void SeekTable::set_point(unsigned index, const ::FLAC__StreamMetadata_SeekPoint &point) { FLAC__ASSERT(is_valid()); @@ -465,6 +471,42 @@ namespace FLAC { return (bool)::FLAC__metadata_object_seektable_is_legal(object_); } + bool SeekTable::template_append_placeholders(unsigned num) + { + FLAC__ASSERT(is_valid()); + return (bool)::FLAC__metadata_object_seektable_template_append_placeholders(object_, num); + } + + bool SeekTable::template_append_point(FLAC__uint64 sample_number) + { + FLAC__ASSERT(is_valid()); + return (bool)::FLAC__metadata_object_seektable_template_append_point(object_, sample_number); + } + + bool SeekTable::template_append_points(FLAC__uint64 sample_numbers[], unsigned num) + { + FLAC__ASSERT(is_valid()); + return (bool)::FLAC__metadata_object_seektable_template_append_points(object_, sample_numbers, num); + } + + bool SeekTable::template_append_spaced_points(unsigned num, FLAC__uint64 total_samples) + { + FLAC__ASSERT(is_valid()); + return (bool)::FLAC__metadata_object_seektable_template_append_spaced_points(object_, num, total_samples); + } + + bool SeekTable::template_append_spaced_points_by_samples(unsigned samples, FLAC__uint64 total_samples) + { + FLAC__ASSERT(is_valid()); + return (bool)::FLAC__metadata_object_seektable_template_append_spaced_points_by_samples(object_, samples, total_samples); + } + + bool SeekTable::template_sort(bool compact) + { + FLAC__ASSERT(is_valid()); + return (bool)::FLAC__metadata_object_seektable_template_sort(object_, compact); + } + // // VorbisComment::Entry ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev