[flac-dev] [PATCH] stream_encoder : Improve selection of residual accumulator width

[lvqcl]

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 6f7ec60c and 187e596e.
>>
>> Sorry I wasn't able to deal with this patch when it came in because I
>> was busy with $day_job.
>>
>> There was a lot of discussion about this patch but I can't really
>> figure out from the thread what the conclusion was.
>>
>> The patch still applies, and the test suite passes. If there is a
>> problem with this patch that the test suite doesn't catch, we should
>> write a test that does catch it.
>>
>> If there is no problem with the patch I'll push it.
>
> As I see it:
>
> 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[]) function.
>
> And indeed, max. possible residual value is 16 times bigger than max. value
> in data[] array:
>
>      residual[i] = data[i] - 4*data[i-1] + 6*data[i-2] - 4*data[i-3] + data[i-4]
>
> 16 == 2^4, so max. bps value for residual[] is equal to max. bps for data[] + 4.
> The value of FLAC__MAX_EXTRA_RESIDUAL_BPS == 4 is enough to fix this problem
> with FLAC__fixed_compute_residual().


On the other hand, it is possible to set FLAC__MAX_EXTRA_RESIDUAL_BPS
to 0, and change evaluate_fixed_subframe_() function instead: in the call
of find_best_partition_order_() function, replace subframe_bps
with subframe_bps + order.


...And maybe it's also better to use 'subframe_bps + order' instead of 'order'
in calculation of 'estimate' variable in evaluate_fixed_subframe_()?