Alex Converse <[email protected]> writes:
> 2012/1/31 Måns Rullgård <[email protected]>:
>> Alex Converse <[email protected]> writes:
>>
>>> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>>> ---
>>> libavcodec/ac3dsp.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/libavcodec/ac3dsp.c b/libavcodec/ac3dsp.c
>>> index 98c7357..8ff9f7d 100644
>>> --- a/libavcodec/ac3dsp.c
>>> +++ b/libavcodec/ac3dsp.c
>>> @@ -125,7 +125,7 @@ static void ac3_bit_alloc_calc_bap_c(int16_t *mask,
>>> int16_t *psd,
>>> int address = av_clip((psd[bin] - m) >> 5, 0, 63);
>>> bap[bin] = bap_tab[address];
>>> }
>>> - } while (end > ff_ac3_band_start_tab[band++]);
>>> + } while (end > ff_ac3_band_start_tab[++band]);
>>> }
>>
>> Won't this (untested) do the same thing while simplifying a little:
>>
>> diff --git a/libavcodec/ac3dsp.c b/libavcodec/ac3dsp.c
>> index 98c7357..f97d424 100644
>> --- a/libavcodec/ac3dsp.c
>> +++ b/libavcodec/ac3dsp.c
>> @@ -108,7 +108,7 @@ static void ac3_bit_alloc_calc_bap_c(int16_t *mask,
>> int16_t *psd,
>> int snr_offset, int floor,
>> const uint8_t *bap_tab, uint8_t *bap)
>> {
>> - int bin, band;
>> + int bin, band, band_end;
>>
>> /* special case, if snr offset is -960, set all bap's to zero */
>> if (snr_offset == -960) {
>> @@ -120,12 +120,12 @@ static void ac3_bit_alloc_calc_bap_c(int16_t *mask,
>> int16_t *psd,
>> band = ff_ac3_bin_to_band_tab[start];
>> do {
>> int m = (FFMAX(mask[band] - snr_offset - floor, 0) & 0x1FE0) + floor;
>> - int band_end = FFMIN(ff_ac3_band_start_tab[band+1], end);
>> + band_end = FFMIN(ff_ac3_band_start_tab[++band], end);
> We clip band here
>> for (; bin < band_end; bin++) {
>> int address = av_clip((psd[bin] - m) >> 5, 0, 63);
>> bap[bin] = bap_tab[address];
>> }
>> - } while (end > ff_ac3_band_start_tab[band++]);
>> + } while (end > band_end);
> Making this always false
>> }
That's not what's wrong with it. The clipping only changes the value
when the condition would anyway be false.
However, it is broken because the argument to FFMIN() has side-effects.
This is correct:
diff --git a/libavcodec/ac3dsp.c b/libavcodec/ac3dsp.c
index 98c7357..bd4d6b9 100644
--- a/libavcodec/ac3dsp.c
+++ b/libavcodec/ac3dsp.c
@@ -108,7 +108,7 @@ static void ac3_bit_alloc_calc_bap_c(int16_t *mask, int16_t
*psd,
int snr_offset, int floor,
const uint8_t *bap_tab, uint8_t *bap)
{
- int bin, band;
+ int bin, band, band_end;
/* special case, if snr offset is -960, set all bap's to zero */
if (snr_offset == -960) {
@@ -120,12 +120,13 @@ static void ac3_bit_alloc_calc_bap_c(int16_t *mask,
int16_t *psd,
band = ff_ac3_bin_to_band_tab[start];
do {
int m = (FFMAX(mask[band] - snr_offset - floor, 0) & 0x1FE0) + floor;
- int band_end = FFMIN(ff_ac3_band_start_tab[band+1], end);
+ band_end = ff_ac3_band_start_tab[++band];
+ band_end = FFMIN(band_end, end);
for (; bin < band_end; bin++) {
int address = av_clip((psd[bin] - m) >> 5, 0, 63);
bap[bin] = bap_tab[address];
}
- } while (end > ff_ac3_band_start_tab[band++]);
+ } while (end > band_end);
}
static void ac3_update_bap_counts_c(uint16_t mant_cnt[16], uint8_t *bap,
--
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
