On 21.12.2016 13:46, wm4 wrote:
> On Wed, 21 Dec 2016 01:43:46 +0100
> Andreas Cadhalpun <andreas.cadhal...@googlemail.com> wrote:
>> On 20.12.2016 15:22, wm4 wrote:
>>> On Mon, 19 Dec 2016 23:36:11 +0100
>>> Andreas Cadhalpun <andreas.cadhal...@googlemail.com> wrote:
>>>> On 16.12.2016 17:22, wm4 wrote:
>>>>> Are you sure we need the message?
>>>>
>>>> Yes, since such an overflow could just be a sign of a limitation in our
>>>> framework (think of bit_rate being int32_t) and does not necessarily mean
>>>> that the sample is invalid.
>>>>
>>>>> It's quite ugly.
>>>>
>>>> Do you have any suggestions for improving it?
>>>
>>> I'm pretty much against such macros for rather specific use-cases, and
>>> putting them into a public headers.
>>
>> It is added to an "internal and external API header".
>> Feel free to send patches separating it into an internal and a public header.
>
> Macros starting with FF are public API,
No, public macros start with AV.
> so don't put that macro in a public header. Or we're stuck with it forever.
>
>>> I'm thinking it'd be better to actually provide overflow-checking
>>> primitives,
>>
>> Why?
>
> Because that would have actual value,
That's a meaningless argument.
> since overflowing checks are annoying
Using overflow-checking primitives is even more annoying.
> and there's also a chance to get them wrong.
That's always the case with anything.
> The code gets less ugly too.
Rather the contrary. Compare
FF_RETURN_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2)
st->codecpar->bits_per_coded_sample = ea->bytes * 8;
st->codecpar->bit_rate = (int64_t)st->codecpar->channels *
st->codecpar->sample_rate *
st->codecpar->bits_per_coded_sample / 4;
st->codecpar->block_align = st->codecpar->channels *
st->codecpar->bits_per_coded_sample;
with:
ret = ff_mul_check_overflow(&st->codecpar->bits_per_coded_sample,
ea->bytes, 8)
if (ret < 0)
return ret;
st->codecpar->bit_rate = (int64_t)st->codecpar->channels *
st->codecpar->sample_rate *
st->codecpar->bits_per_coded_sample / 4;
ret = ff_mul_check_overflow(&st->codecpar->block_align,
st->codecpar->channels, st->codecpar->bits_per_coded_sample)
if (ret < 0)
return ret;
> If you're going to add such overflow checks to every
> single operation in libavcodec that could overflow, you better come up
> with something that won't add tons of crap to the code.
Code that overflows is "crap", not the checks preventing that.
>>> and I also don't think every overflow has to be logged.
>>
>> I disagree for the reason I mentioned above.
>
> Which was? Not going to read the whole thread again.
Reading either of the mails you replied to would have been sufficient.
You've got a third chance with this mail.
Best regards,
Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
