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 <mich...@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, Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel