[FFmpeg-devel] [PATCH V4 1/2] lavfi/nlmeans: Checking number precision when computing integral images

[mypopy at gmail.com]

On Fri, Mar 8, 2019 at 5:26 PM Paul B Mahol <onemda at gmail.com> wrote:
>
> On 3/8/19, Jun Zhao <mypopydev at gmail.com> wrote:
> > From: Jun Zhao <barryjzhao at tencent.com>
> >
> > accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits (uint32_t *ii)
> > data type, it will have a risk of an integral value becoming larger than
> > the 32-bits integer capacity and resulting in an integer overflow. For
> > this risk, add a checking with warning message.
> >
> > Signed-off-by: Jun Zhao <barryjzhao at tencent.com>
> > ---
> >  libavfilter/vf_nlmeans.c |   11 +++++++++++
> >  1 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
> > index dcb5a03..8d47f9d 100644
> > --- a/libavfilter/vf_nlmeans.c
> > +++ b/libavfilter/vf_nlmeans.c
> > @@ -477,6 +477,17 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> > *in)
> >      NLMeansContext *s = ctx->priv;
> >      AVFilterLink *outlink = ctx->outputs[0];
> >
> > +    // accumulation of 8-bits uint_8 into 32-bits data type, it will have
> > +    // a risk of an integral value becoming larger than the 32-bits integer
> > +    // capacity and resulting in an integer overflow, so limit the image
> > size
> > +    if ((UINT32_MAX / (uint64_t)inlink->w) < (255 * (uint64_t)inlink->h)) {
> > +        av_log(ctx, AV_LOG_ERROR,
> > +               "image size (%d x %d) integral value may overflow.\n",
> > +               inlink->w, inlink->h);
> > +        av_frame_free(&in);
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> >      AVFrame *out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> >      if (!out) {
> >          av_frame_free(&in);
>
> I see no point in this warning, if overflow is real issue should be
> fixed instead of giving
> pointless warning.
In fact, this is a potential overflow problems depend on image
value/width/height when calculating integral image(Summed-area_table
is the other name https://en.wikipedia.org/wiki/Summed-area_table),
this is the reason to limit the image size in this patch to avoid this
potential overflow problems, I don't know what's the mean for " should
be fixed instead of giving pointless warning.", can you give more
information for this? thx.