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

Andreas Cadhalpun andreas.cadhalpun at googlemail.com
Thu Nov 3 00:09:17 EET 2016

On 26.10.2016 21:44, Andreas Cadhalpun wrote:
> 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.)

In the absence of further comments, I intend to push this set in a few days.

Best regards,

More information about the ffmpeg-devel mailing list