similar to: [PATCH] stream_encoder : Improve selection of residual accumulator width

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 +

Here I attach a preliminary version of a patch for precompute_partition_info_sums_() function that should accelerate encoding of 24-bit input data. 1) SSE2, SSSE3 and AVX2 versions of this function should be updated, too 2) the patch also changes if(FLAC__bitmath_ilog2(default_partition_samples) + bps + FLAC__MAX_EXTRA_RESIDUAL_BPS < 32) into (in effect)

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

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 &&

Anybody still has MSVC 2015 update 1? It seems that MSVC 2015U2 miscompiles stream_encoder_intrin_sse2.c, stream_encoder_intrin_ssse3.c (and probably stream_encoder_intrin_avx2.c, but currently I cannot test it 'cause I don't have avx2 CPU). I wonder what happened and what changed in the asm code that MSVC generates.

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)

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

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)

Erik de Castro Lopo wrote: > Ok, I'll take the original patch as it is. DO you have a link to a > bug report on this problem? It would be nice to put it in a comment > above the workaround. :sigh: I hoped that somebody else will find what's wrong and will create a bugreport... Well, here is the link:

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: > >

Martijn van Beurden wrote: > You've exposed at least two very serious FLAC bugs, > namely a malfunctioning RICE2-partition encoder and a bug concerning > choosing verbatim frames over fixed/lpc frames. The second, not choosing verbatim frames over fixed/lpc frames is almost certainly a direct result of the first problem. The fix was changing one local variable from FLAC_uint32 to

--- 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 +++---------

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:

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:

Here's a new version of a patch that fixes a problem with MSVC2105 update2, but it doesn't disable any optimization, so the resulting encoding performance should be almost unaffected by this workaround. MSVC compiles abs_residual_partition_sums[partition] = (FLAC__uint32)_mm_cvtsi128_si32(mm_sum); into this: movq QWORD PTR [rsi], xmm2 while it should be movd

Erik de Castro Lopo wrote: > Both applied. Thanks. Thanks! But for some reason stream_encoder_intrin_sse2.c wasn't updated by the 2nd patch...

Erik de Castro Lopo <mle+la at mega-nerd.com> wrote: >> -msse for SSE code, -msse2 for SSE2 code, -msse4.1 for SSE4.1 code > > Yes, that was it. Brain fart. These flags were not needed on x86_64. > > Erik But now all C code is compiled with -msse2 and it won't work on older CPUs. Isn't it better to compile only necessary files with this flag?