Hi, 2012/3/31 Måns Rullgård <[email protected]>: > "Ronald S. Bultje" <[email protected]> writes: >> On Sat, Mar 31, 2012 at 9:18 AM, Justin Ruggles >> <[email protected]> wrote: >>> On 03/29/2012 03:44 PM, Ronald S. Bultje wrote: >>>> From: "Ronald S. Bultje" <[email protected]> >>>> This prevents sample_rate/data_length from going negative, which >>>> caused various crashes and undefined behaviour further down. >>>> >>>> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind >>>> CC: [email protected] >>>> --- >>>> libavcodec/tta.c | 8 +++++--- >>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/libavcodec/tta.c b/libavcodec/tta.c >>>> index ad80246..808de04 100644 >>>> --- a/libavcodec/tta.c >>>> +++ b/libavcodec/tta.c >>>> @@ -61,7 +61,8 @@ typedef struct TTAContext { >>>> GetBitContext gb; >>>> const AVCRC *crc_table; >>>> >>>> - int format, channels, bps, data_length; >>>> + int format, channels, bps; >>>> + unsigned data_length; >>>> int frame_length, last_frame_length, total_frames; >>>> >>>> int32_t *decode_buffer; >>>> @@ -253,7 +254,7 @@ static av_cold int tta_decode_init(AVCodecContext * >>>> avctx) >>>> } >>>> >>>> // prevent overflow >>>> - if (avctx->sample_rate > 0x7FFFFF) { >>>> + if (avctx->sample_rate > 0x7FFFFFu) { >>> >>> >>> this change doesn't make sense to me >> >> Samplerate is a 32bit thing in the bitstream header (read in the >> codec, not the demuxer), placed in a signed integer. Therefore, >> 0x80000000 and up are read as negative. Comparing to an unsigned int >> yields an unsigned comparison which errors out on negatives. > > Does it make sense at all for AVCodecContext.sample_rate to be signed?
I've asked that question before, and never got a satisfactory answer. I suppose we could consider changing it, perhaps under an ABI bump ifdef or stuff like that if we feel uncomfortable with API breakage potential? Ronald _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
