similar to: Duplicate QLP coefficient restricting code

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

When using the -p switch during encoding, the encoder should try different qlp predictor precision steps. However, some faulty code was too severely restricting the possible steps. This patch lifts the restriction to match a restriction coded a little further in the process. This doesn't make using -p worth your while, but at least it doesn't create larger files now ---

Ok, I just did a comparison of 1.2.1 with 1.3.2, and the change you're suggesting was already there before. So, now the question becomes: why was the code changed in the first place? Was there a bug that was fixed by changing 17 to 16, or did someone just get overzealous in a code review and thought that 17 was a bad choice? Perhaps 32 bits isn't actually large enough to handle the

stream_encoder.c has the following code: /* try to keep qlp coeff precision such that only 32-bit math is required for decode of <=16bps streams */ if(subframe_bps <= 16) { ... But FLAC can convert 16-bit input to 17-bit if mid-side coding is used. So, does it make sense to compare subframe_bps with 17? (The patch is attached. What do you think about it?) -------------- next part

Martijn van Beurden wrote: > Yes, but that MAX value is used to loop over the > qlp_coeff_precision values between MIN and MAX. So, if the > qlp_coeff_precision value is limited in the loop but MAX is not > limited, the loop does the exact same thing multiple times: a > waste of time. Therefore, only the MAX should be limited. > > I don't think the logic is needed

Martijn van Beurden wrote: > Or maybe the question is: why is this code in evaluate_lpc_subframe anyway, > i.e, why is this code duplicated? If process_subframe_ sets the > qlp_precision for evaluate_lpc_subframe, why should the latter ignore that? > > We can only speculate about this, but I think this code has been duplicated > by accident, and therefore it wasn't changed

Martijn van Beurden wrote: > Yes, indeed. I removed the 17-bits part because I just matched > the code in evaluate_lpc_subframe_ with the process_subframe_ > code. It appears it only makes sense that those two pieces code > are the same. A bit of history: 1) The conditional "if(subframe_bps <= 16)" was added to evaluate_lpc_subframe_() in the commit

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

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)

libFLAC limits the value of qlp_coeff_precision to make sure that 32-bit math is enough for decoding of 16-bit audio. subframe_bps can be equal to 17 for 16-bit input (for side channel). So the value of subframe_bps should be compared with 17, not 16 (see also http://git.xiph.org/?p=flac.git;a=blob;f=src/libFLAC/stream_encoder.c;hb=02591f6b4b09e7c6e26bd2ae7225ff2810b227bb#l3431 ) --------------

On Sat, May 19, 2001 at 12:55:08AM -0400, Matt Zimmerman wrote: > On Sat, May 19, 2001 at 02:05:14AM +0000, Christian Weisgerber wrote: > > > Problems in FLAC 0.9: > > > > - On alpha, flac immediately dumps core for both encoding and > > decoding (FreeBSD/alpha). > > I have reproduced this on Debian/alpha as well. I will spend some time > debugging it

On Sun, Jun 24, 2001 at 02:30:56PM -0700, Josh Coalson wrote: > There have been reports of -9 using huge amounts of memory. -9 is really > theoretical, but people always seem to want to try the max setting. Anyway, > that's not an excuse but figuring out why -9 is using so much memory is lower > on my list than other stuff. -8 should get within 0.01% of -9 and is pretty >

Erik de Castro Lopo wrote: > There should be some indication of why in the git history. http://git.xiph.org/?p=flac.git;a=commitdiff;h=27846708fe6271e5e3965a4bbad99baa1ca24c49 Now I remember a discussion about a bug in -p switch: the old code substracts lpc_order instead of FLAC__bitmath_ilog2(lpc_order), and this commit fixes this. It seems that the logic in process_subframe_() and in

Matt Zimmerman <mdz@debian.org> wrote: > Aha. In FLAC__fixed_restore_signal, the index variable 'i' is declared > unsigned, then used like so: [...] With the following trivial patch applied, > everything works on Alpha, at least with my test sample. Against 0.9 or CVS? While this clearly fixes a bug, 0.9 still dies for me. #0 0x1600692a8 in

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

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[])

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

Hi! Thanks for maintaining FLAC! I recently started a project to compile parts of the FLAC reference distribution to JavaScript using emscripten. https://github.com/gagern/emflac#readme I'm using the 1.3.1 release and the Makefile.lite build process. I'm including the FLAC reference implementation as a submodule, and so far I managed to compile everything without modifying its