similar to: "keep qlp coeff precision such that only 32-bit math is required"

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

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

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

Hi all, I was investigating the behaviour of the -p switch of flac. This switch should enable exhaustive search for the optimal qlp coefficient precision, but the resulting files are usually 0.5% larger then when not using the switch. I stumbled upon this code in stream_encoder > if(encoder->protected_->do_qlp_coeff_prec_search) { > min_qlp_coeff_precision =

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

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

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

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

Brian Willoughby wrote: > 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? There should be some indication of why in the git history. Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo

On Thu, 2007-04-05 at 02:27 -0700, Brian Willoughby wrote: > Josh (Green), > > Seems like the longest example in your list is a 15-second file. I > would like to see the same problem exhibited in a file that is of a > normal length. I have been recording full performances lasting > hours, and flac always compresses the files below 70% of the original > size. >

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)

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)

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

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

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

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

Problems in FLAC 0.9: - On alpha, flac immediately dumps core for both encoding and decoding (FreeBSD/alpha). - The distribution Makefile.in files haven't been generated with "automake --include-deps". The resulting Makefiles aren't fully portable; in particular they break with BSD make. In the future, care should be taken to use "--include-deps". - What is

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

> > I also had this verify error encoding a wav I ripped from a CD. I > didn't > > report this as it happended on flac running on debian linux > -current unstable. > > This error happened only with one track of a CD I was ripping. > > > > Another reason for me not reporting this was, that flac 1.0 running > on OpenBSD > > 2.9 encoded the wav with