[FFmpeg-devel] [PATCH] af_volume: fix integer clip

Zhao Zhili quinkblack at foxmail.com
Mon Jan 27 12:28:09 EET 2020



> On Jan 27, 2020, at 12:59 AM, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
> 
> Am So., 26. Jan. 2020 um 17:13 Uhr schrieb Zhao Zhili <quinkblack at foxmail.com <mailto:quinkblack at foxmail.com>>:
>> 
>> ---
>> Or specify an upper limit on volume. What do you think?
>> 
>> libavfilter/af_volume.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c
>> index 213c57195a..029925cbfb 100644
>> --- a/libavfilter/af_volume.c
>> +++ b/libavfilter/af_volume.c
>> @@ -200,7 +200,7 @@ static inline void scale_samples_s16(uint8_t *dst, const uint8_t *src,
>>     int16_t *smp_dst       = (int16_t *)dst;
>>     const int16_t *smp_src = (const int16_t *)src;
>>     for (i = 0; i < nb_samples; i++)
>> -        smp_dst[i] = av_clip_int16(((int64_t)smp_src[i] * volume + 128) >> 8);
>> +        smp_dst[i] = (int16_t)av_clip64(((int64_t)smp_src[i] * volume + 128) >> 8, INT16_MIN, INT16_MAX);
> 
> The cast looks unnecessary and confusing but if a limit works, it is likely
> simpler imo.

The function is supposed to handle smp_src[i] * volume > INT32_MAX, so the cast is necessary.

    case AV_SAMPLE_FMT_S16:
        if (vol->volume_i < 0x10000)
            vol->scale_samples = scale_samples_s16_small;
        else
            vol->scale_samples = scale_samples_s16;
        break;

I'm not sure about the use case. Does the function suppose to handle
(((int64_t)smp_src[i] * volume + 128) >> 8) > INT32_MAX?

By the way, there is another overflow in set_volume():

    if (vol->precision == PRECISION_FIXED) {
        vol->volume_i = (int)(vol->volume * 256 + 0.5);
        vol->volume   = vol->volume_i / 256.0;
        av_log(ctx, AV_LOG_VERBOSE, "volume_i:%d/255 ", vol->volume_i);
    }

> 
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org <mailto:ffmpeg-devel-request at ffmpeg.org> with subject "unsubscribe".



More information about the ffmpeg-devel mailing list