On Fri, Mar 13, 2015 at 1:26 PM, Luca Barbato <[email protected]> wrote: > On 13/03/15 14:11, Vittorio Giovara wrote: >> >> On Fri, Mar 13, 2015 at 10:14 AM, Luca Barbato <[email protected]> wrote: >>> >>> On 13/03/15 10:42, Diego Biurrun wrote: >>>> >>>> On Fri, Mar 13, 2015 at 02:21:23AM +0100, Luca Barbato wrote: >>>>> >>>>> Avoid spurious dimension check messages that the parser might trigger. >>>>> --- a/libavcodec/pnm.c >>>>> +++ b/libavcodec/pnm.c >>>>> @@ -140,6 +140,8 @@ int ff_pnm_decode_header(AVCodecContext *avctx, >>>>> PNMContext * const s) >>>>> return AVERROR_INVALIDDATA; >>>>> pnm_get(s, buf1, sizeof(buf1)); >>>>> avctx->height = atoi(buf1); >>>>> + if (avctx->height <= 0) >>>>> + return AVERROR_INVALIDDATA; >>>>> if(av_image_check_size(avctx->width, avctx->height, 0, avctx)) >>>>> return AVERROR_INVALIDDATA; >>>> >>>> >>>> WTH? Why are the checks duplicated? av_image_check_size validates >>>> width >>>> and height?!? >>>> >>> >>> Because otherwise the false positives when used from the parser would >>> trigger reports from concerned users. >> >> >> Could you describe the usecase bettter? I feel like either of the two >> size checks could be dropped. >> > That function is used in 2 places: > > - in the parser, to cut the single images out of a stream of them > - in the decoder to read it > > The function av_image_check_size loudly report the error causing reports > about something not working (nocebo effect?). > > Actually it is possible to have also the other corner cases while trying to > split the images so might be a good idea to have a quiet variant of that > check function and just use it.
I think I'd rather have an extra parameter to check if it's being called from parser or not and fail (or not) appropriately. In either case this needs a couple of lines of comments about it. -- Vittorio _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
