2012/1/31 Måns Rullgård <[email protected]>: > 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, > > --
OK _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
