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

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 as the code shouldn't
have
> been there in the first place.
Well, the code in process_subframe_() calculates MAX value of
qlp_coeff_precision,
while the code in evaluate_lpc_subframe_() adjusts CURRENT qlp_coeff_precision.

It's possible to move the logic from evaluate_lpc_subframe_() into
process_subframe_(), see the attached patch, but I'm not sure that the
code becomes much clearer after it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tmp1.patch
Type: application/octet-stream
Size: 1518 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20150420/741a6064/attachment.obj

Op 20-04-15 om 18:45 schreef lvqcl:
> Well, the code in process_subframe_() calculates MAX value of 
> qlp_coeff_precision,
> while the code in evaluate_lpc_subframe_() adjusts CURRENT 
> qlp_coeff_precision.
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 directly after
> min_qlp_coeff_precision = max_qlp_coeff_precision =
encoder->protected_->qlp_coeff_precision;
because that value is either chosen by the encoder (it is a 
lookup value, and is always small enough not to trigger this, 
set in stream_encoder.c around line 700) or it is set through 
the encoder, in which case it should probably not be overriden 
without notice.

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 directly after
>> min_qlp_coeff_precision = max_qlp_coeff_precision =
encoder->protected_->qlp_coeff_precision;
> because that value is either chosen by the encoder (it is a
> lookup value, and is always small enough not to trigger this,
> set in stream_encoder.c around line 700) or it is set through
> the encoder, in which case it should probably not be overriden
> without notice.
Currently evaluate_lpc_subframe_() automatically adjusts qlp_coeff_precision
when a user sets non-standard order value (via -l option). I don't think
that it's incorrect behavior. Why not?

(And my initial point was that the current limit of qlp_coeff_precision is not
enough,
and 32-bit datapath is *not* ensured for 16-bit input audio).