flac dev - Apr 2015 - "keep qlp coeff precision such that only 32-bit math is required"

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 evaluate_lpc_subframe_()
is the same, so the constants in the conditions should be the same: either
both equal to 16, or both equal to 17.



I built two flac executables, the 1st is from current git head, the 2nd - with
my patch applied. Then I took a 16-bit WAV file and compressed it with both
encoders.

Standard compression levels (-0...-8): no difference (the resulting files are
bit-identical).
-p option: no difference (the resulting files are bit-identical).
-q N option (N = 13...15): the 2nd encoder produces slightly smaller files.
-l N --lax option (N = 16...31): again, the 2nd encoder produces slightly
smaller files.

About decoding speed: the difference isn't very big.
Except for "-l N --lax" option and 32-bit decoder: the files created
by the 2nd
encoder have noticeably faster decompression speed.

So it seems that my patch is slightly beneficial for those who use
non-standard encoding presets. Especially when they use non-Subset
encoding options.

Op 18-04-15 om 22:56 schreef lvqcl:
>
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.
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. As the evaluate_lpc_subframe code worked correctly 
(for the -p switch problem) and the process_subframe_code 
didn't, I replaced it.

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
http://git.xiph.org/?p=flac.git;a=commitdiff;h=20ac2c1cac298b8c0666e191ae913f6481e79879


2) The conditional "if(subframe_bps <= 16)" was added to
process_subframe_()
in the commit
http://git.xiph.org/?p=flac.git;a=commitdiff;h=c9c0d130c562044e641e2cab09894213c5de4d88


3) The conditional in process_subframe_() was changed to "if(subframe_bps
<= 17)"
in the commit
http://git.xiph.org/?p=flac.git;a=commitdiff;h=df59845d04a3c43e3caea5d9838d08b9b17ba572

So the question is: why the condition in evaluate_lpc_subframe_() wasn't
changed in this commit.
Maybe there were some reasons, but maybe Josh forgot about it.