similar to: A patch for precompute_partition_info_sums_()

In the precompute_partition_info_sums_ function, instead of selecting 64-bit accumulator when the signal bps is larger than 16, revert to the original approach based on partition size, but make room for few extra bits to not overflow with unusual signals where the average residual magnitude may be larger than bps. It slightly improves the performance with standard encoding levels and 16-bit files

The commit http://git.xiph.org/?p=flac.git;a=commitdiff;h=0a0a10f358345f749e4a05021301461994f1ffc5 (from 31 Mar 2007) adds the following conditional: if(FLAC__bitmath_ilog2(default_partition_samples) + bps < 32) And the commit http://git.xiph.org/?p=flac.git;a=commitdiff;h=f081524c19eeafd08f4db6ee5d52a9634c60f475 has this: if(FLAC__bitmath_ilog2(default_partition_samples) +

On Fri, Jun 20, 2014 at 01:21:03PM +0400, lvqcl wrote: > Miroslav Lichvar ?????: > > > +/* > > + * This is used to avoid overflow with unusual signals in 32-bit > > + * accumulator in the *precompute_partition_info_sums_* functions. > > + */ > > +#define FLAC__MAX_EXTRA_RESIDUAL_BPS 4 > > > + /* WATCHOUT: "+ bps +

On Mon, Jun 30, 2014 at 03:27:18AM +0400, lvqcl wrote: > lvqcl wrote: > > FLAC 1.2.1 and 1.3.0 cannot encode snippet6.wav with -7 and -8 encoding modes. > > But they are able to do this with --disable-fixed-subframes option. This > > implies that snippet6.wav triggers a problem somewthere inside > > FLAC__fixed_compute_residual(data[], data_len, order, residual[])

On Thu, Jun 19, 2014 at 06:25:57PM +0400, lvqcl wrote: > Now I wonder why evaluate_lpc_subframe_() function in stream_encoder.c contains > almost the same code, but without any comments that it's not enough pessimistic: > evaluate_lpc_subframe_(): > > if(subframe_bps + qlp_coeff_precision + FLAC__bitmath_ilog2(order) <= 32) > if(subframe_bps <= 16 &&

I changed the condition in *_precompute_partition_info_sums_*() functions from if(bps <= 16) to if(FLAC__bitmath_ilog2(default_partition_samples) + bps < 32) and then changed the 'subframe_bps' argument of find_best_partition_order_() in evaluate_fixed_subframe_() and evaluate_lpc_subframe_() as follows: evaluate_fixed_subframe_(): evaluate_lpc_subframe_(): 1)

lvqcl wrote: > I changed the condition in *_precompute_partition_info_sums_*() > functions from > if(bps <= 16) > to > if(FLAC__bitmath_ilog2(default_partition_samples) + bps < 32) > > and then changed the 'subframe_bps' argument of find_best_partition_order_() > in evaluate_fixed_subframe_() and evaluate_lpc_subframe_() as follows: > >

Miroslav Lichvar wrote: > On Wed, Jul 17, 2013 at 07:45:53PM +1000, Erik de Castro Lopo wrote: > > The fix was changing one local variable from FLAC_uint32 to FLAC_uint64 > > in function precompute_partition_info_sums_(). > > > > https://git.xiph.org/?p=flac.git;a=commit;h=6f7ec60c7e7f05f5ab0b1cf6b7b0945e44afcd4b > > I don't like this fix. It will

On Thu, Jun 19, 2014 at 03:30:22PM +0400, lvqcl wrote: > BTW, what can you say about the following place in stream_decoder.c > in read_subframe_lpc_() function: > > /*@@@@@@ technically not pessimistic enough, should be more like > if( (FLAC__uint64)order * ((((FLAC__uint64)1)<<bps)-1) * ((1<<subframe->qlp_coeff_precision)-1) < (((FLAC__uint64)-1)

Erik de Castro Lopo wrote: >> It slightly improves the performance with standard encoding levels and >> 16-bit files as the 17-bit side channel can still be processed with the >> 32-bit accumulator and correctly selects the 64-bit accumulator with >> very large 16-bit partitions. >> >> This is related to commits 6f7ec60c and 187e596e. > > Sorry I

This patch slightly changes precompute_partition_info_sums_ set of functions. It doesn't alter the logic of these functions. The idea is to simplify them to make the following patches more simpler (I still try to increase the calculations for 24-bit audio). -------------- next part -------------- A non-text attachment was scrubbed... Name: precomp_partitions_1.patch Type:

I don't understand the comment in src/libFLAC/stream_decoder.c: /*@@@@@@ technically not pessimistic enough, should be more like if( (FLAC__uint64)order * ((((FLAC__uint64)1)<<bps)-1) * ((1<<subframe->qlp_coeff_precision)-1) < (((FLAC__uint64)-1) << 32) ) */ if(bps + subframe->qlp_coeff_precision + FLAC__bitmath_ilog2(order) <= 32) see

--- configure.ac | 7 +++++ src/libFLAC/bitreader.c | 12 ++------- src/libFLAC/bitwriter.c | 8 ++---- src/libFLAC/fixed.c | 18 +++++-------- src/libFLAC/format.c | 8 ++---- src/libFLAC/include/private/macros.h | 29 ++++++++++++++++++++ src/libFLAC/metadata_iterators.c | 17 +++---------

lvqcl wrote: > Erik de Castro Lopo wrote: > >>> It slightly improves the performance with standard encoding levels and >>> 16-bit files as the 17-bit side channel can still be processed with the >>> 32-bit accumulator and correctly selects the 64-bit accumulator with >>> very large 16-bit partitions. >>> >>> This is related to commits

On Mon, Apr 20, 2015 at 08:00:20PM +0300, lvqcl wrote: > I don't understand the comment in src/libFLAC/stream_decoder.c: > > /*@@@@@@ technically not pessimistic enough, should be more like > if( (FLAC__uint64)order * ((((FLAC__uint64)1)<<bps)-1) * ((1<<subframe->qlp_coeff_precision)-1) < (((FLAC__uint64)-1) << 32) ) > */ > if(bps +

I have noticed lpc slowdown both in encoding and decoding, not related to new config.h stuff. It seems there is wrong choosing of fastest possible version of lpc function. Patch is attached. -- Miroslav Lichvar -------------- next part -------------- Index: src/libFLAC/stream_decoder.c =================================================================== RCS file:

Miroslav Lichvar wrote: > In the precompute_partition_info_sums_ function, instead of selecting > 64-bit accumulator when the signal bps is larger than 16, revert to the > original approach based on partition size, but make room for few extra > bits to not overflow with unusual signals where the average residual > magnitude may be larger than bps. > > It slightly improves

All(?) non-static functions have FLAC__ prefix. But precompute_partition_info_sums_32bit_asm_ia32_() and ..._intrin_sse2() and ..._intrin_ssse3() don't have it. This patch adds the prefix to them. -------------- next part -------------- A non-text attachment was scrubbed... Name: FLAC__prefix.patch Type: application/octet-stream Size: 6396 bytes Desc: not available Url :

Here's what I listed in that email. Merging doesn't appear to be necessary. If you have any build problems, let me know. Note that my detection code is Darwin-specific. It's a BSD call (sysctl()), so a change to the platform-detection macros should enable it to work on other BSDs. However, I don't know what that would be, and I couldn't determine any safe way to do the check

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