[FFmpeg-devel] [PATCH 01/12] adxdec: validate sample_rate

[Andreas Cadhalpun]

On 26.10.2016 20:15, Paul B Mahol wrote:
> On 10/25/16, Michael Niedermayer <michael at niedermayer.cc> wrote:
>> On Tue, Oct 25, 2016 at 07:45:25PM +0200, Andreas Cadhalpun wrote:
>>> On 25.10.2016 12:58, Paul B Mahol wrote:
>>>> patch(es)have good intent, but better fix is doing/checking it in single
>>>> place.
>>>
>>> I don't agree.
>>> In general, validity checks should be where the values are actually read.
>>> This eliminates the risk that bogus values could cause problems between
>>> being set
>>> and being checked.
>>> Also, having only a check in a central place is bad for debugging, because
>>> it is
>>> not immediately clear where the bogus value came from, when the check is
>>> triggered.
>>> (I know this from personal experience debugging all the cases triggering
>>> the
>>> assert in av_rescale_rnd.)
>>>
>>> The problem with that approach is that such checks can easily be
>>> forgotten, which
>>> is why I think a check in a central place would make sense in addition to
>>> checking
>>> the individual cases.
>>
>> some formats may also lack a sample rate like mpeg ps/ts
>> the fact is known insude the demuxer, generic code after it doesnt
>> know. Doing the only check after the parser is a bit late OTOH
>>
>> [...]
>>
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> What does censorship reveal? It reveals fear. -- Julian Assange
>>
> 
> I'm not (yet) aware of better "fix" so do as you like.

Have you seen the patch for checking this in a central place [1]?
It would catch all the negative sample rates, but not the negative
timescales.
(I still think it should be checked both centrally and locally.)

Best regards,
Andreas


1: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-October/201769.html