On Fri, Oct 29, 2021 at 3:12 PM Tomas Härdin <tjop...@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
