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

[Paul B Mahol]

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

> tor 2021-10-28 klockan 21:09 +0200 skrev Paul B Mahol:
> >
> > +FRAMESYNC_DEFINE_CLASS(wpsnr, WPSNRContext, fs);
> > +
> > +#define COMPUTE_HX(type, stype, depth)               \
> > +static void compute_hx##depth(const uint8_t *ssrc,   \
> > +                              int linesize,          \
> > +                              int w, int h,          \
> > +                              uint16_t *dstp,        \
> > +                              int dst_linesize)      \
> > +{                                                    \
> > +    const type *src = (const type *)ssrc;            \
> > +    stype *dst = (stype *)dstp;                      \
> > +                                                     \
> > +    linesize /= (depth / 8);                         \
>
> Can linesize ever be odd? Probably not, so this should be fine.
>
> > +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.


>
> > +    int64_t sum = 0;
> > +
> > +    linesize /= 2;
> > +
> > +    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);
> > +}
>
> Why not use the same kind of templatization for these as compute_hx*?
>
> > +
> > +static double get_sd(const uint8_t *ref, int ref_linesize,
> > +                     const uint8_t *main, int main_linesize,
> > +                     int w, int h)
> > +{
> > +    int64_t sum = 0;
> > +
> > +    for (int y = 0; y < h; y++) {
> > +        for (int x = 0; x < w; x++)
> > +            sum += pow_2(ref[x] - main[x]);
> > +        ref += ref_linesize;
> > +        main += main_linesize;
> > +    }
> > +
> > +    return sum;
> > +}
> > +
> > +static double get_sd16(const uint8_t *rref, int ref_linesize,
> > +                       const uint8_t *mmain, int main_linesize,
> > +                       int w, int h)
> > +{
> > +    const uint16_t *ref = (const uint16_t *)rref;
> > +    const uint16_t *main = (const uint16_t *)mmain;
> > +    int64_t sum = 0;
> > +
> > +    ref_linesize /= 2;
> > +    main_linesize /= 2;
> > +
> > +    for (int y = 0; y < h; y++) {
> > +        for (int x = 0; x < w; x++)
> > +            sum += pow_2(ref[x] - main[x]);
> > +        ref += ref_linesize;
> > +        main += main_linesize;
> > +    }
> > +
> > +    return sum;
> > +}
>
> Same here, and for more functions in the patch it seems so I'm not
> going to bother repeating myself any more
>
> > +static void set_meta(AVDictionary **metadata, const char *key, char
> > comp, float d)
> > +{
> > +    char value[128];
> > +    snprintf(value, sizeof(value), "%f", d);
> > +    if (comp) {
> > +        char key2[128];
> > +        snprintf(key2, sizeof(key2), "%s%c", key, comp);
> > +        av_dict_set(metadata, key2, value, 0);
> > +    } else {
> > +        av_dict_set(metadata, key, value, 0);
> > +    }
> > +}
>
> We should probably add av_dict_set_* for int, double etc at some point
>
> /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".
>