[FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter

[Paul B Mahol]

On Fri, Oct 29, 2021 at 7:26 PM Tomas Härdin <tjoppen at acc.umu.se> wrote:

> fre 2021-10-29 klockan 19:17 +0200 skrev Paul B Mahol:
> > On Fri, Oct 29, 2021 at 6:59 PM Tomas Härdin <tjoppen at acc.umu.se>
> > wrote:
> >
> > > fre 2021-10-29 klockan 17:00 +0200 skrev Paul B Mahol:
> > > > On Fri, Oct 29, 2021 at 4:46 PM Tomas Härdin <tjoppen at acc.umu.se>
> > > > wrote:
> > > >
> > > > > fre 2021-10-29 klockan 15:33 +0200 skrev Paul B Mahol:
> > > > > > On Fri, Oct 29, 2021 at 3:12 PM Tomas Härdin <
> > > > > > tjoppen at acc.umu.se>
> > > > > > wrote:
> > > > > > >
> > > > > > > > +static double get_hx(const uint8_t *src, int linesize,
> > > > > > > > int
> > > > > > > > w,
> > > > > > > > int h)
> > > > > > > > +{
> > > > > > > > +    int64_t sum = 0;
> > > > > > > > +
> > > > > > > > +    for (int y = 0; y < h; y++) {
> > > > > > > > +        for (int x = 0; x < w; x++) {
> > > > > > > > +            sum += 12 * src[x] -
> > > > > > > > +                    2 * (src[x-1] + src[x+1] +
> > > > > > > > +                         src[x + linesize] +
> > > > > > > > +                         src[x - linesize]) -
> > > > > > > > +                    1 * (src[x - 1 - linesize] +
> > > > > > > > +                         src[x + 1 - linesize] +
> > > > > > > > +                         src[x - 1 + linesize] +
> > > > > > > > +                         src[x + 1 + linesize]);
> > > > > > > > +        }
> > > > > > > > +
> > > > > > > > +        src += linesize;
> > > > > > > > +    }
> > > > > > > > +
> > > > > > > > +    return fabs(sum * 0.25);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static double get_hx16(const uint8_t *ssrc, int
> > > > > > > > linesize,
> > > > > > > > int w,
> > > > > > > > int
> > > > > > > > h)
> > > > > > > > +{
> > > > > > > > +    const uint16_t *src = (const uint16_t *)ssrc;
> > > > > > >
> > > > > > > This is not -fstrict-aliasing safe
> > > > > > >
> > > > > >
> > > > > > How so? I get no warnings at all, and similar is used
> > > > > > everywhere
> > > > > > else.
> > > > >
> > > > > Then those places should be fixed. We have macros like
> > > > > AV_RB16()
> > > > > for a
> > > > > reason. That gcc doesn't warn about this doesn't mean it isn't
> > > > > free
> > > > > to
> > > > > assume ssrc and src points to different non-overlapping regions
> > > > > of
> > > > > memory.
> > > > >
> > > > >
> > > > That is sub optimal and unacceptable solution.
> > >
> > > Did you do measurements to come to this conclusion?
> > >
> > > > Review remark ignored.
> > >
> > > Undefined behavior is *not* acceptable. If you want this file
> > > specifically to be compiled with strict aliasing disabled then you
> > > must
> > > at the very least change the build system accordingly
> > >
> >
> > Cite source that can prove this.
>
> The gcc manpage
>

citations needed.


>
> > Compilers are silent about this.
>
> Irrelevant
>


citations needed.


>
> > There are hundredths if not thousands of such examples across
> > codebase.
>
> This is not a valid excuse. We need less UB in the codebase, not more.
> UBs beget CVEs.
>

citations needed.


>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>