On Tue, Oct 16, 2012 at 02:02:43PM +0200, Luca Barbato wrote:
> The diff method shows the per pixel difference while the dssim method
> also outputs the dssim value.
> ---
>
> now with checked malloc.
:)
> --- /dev/null
> +++ b/libavfilter/compare.h
> @@ -0,0 +1,40 @@
> +/*
> + * This file is part of Libav.
> + *
> + * Libav is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Libav is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with Libav; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
So this header is GPLed? How did this happen, copy and paste from the
wrong file?
> +typedef struct {
> +} CmpContext;
No anonymously typedeffed structs in headers please.
> +#ifndef AVFILTER_DSSIM_H
> +#define AVFILTER_DSSIM_H
> +
> +#endif /* AVFILTER_DSSIM_H */
This does not match the filename.
> --- /dev/null
> +++ b/libavfilter/dssim.c
> @@ -0,0 +1,335 @@
> + *
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials
> + * provided with the distribution.
I've always wondered how many different ones of those we have by now ...
> + fx =
> + ((fx > epsilon) ? powf(fx,
> + (1.0f / 3.0f)) : (7.787f * fx + 16.0f /
> 116.0f));
> + fy =
> + ((fy > epsilon) ? powf(fy,
> + (1.0f / 3.0f)) : (7.787f * fy + 16.0f /
> 116.0f));
> + fz =
> + ((fz > epsilon) ? powf(fz,
> + (1.0f / 3.0f)) : (7.787f * fz + 16.0f /
> 116.0f));
This is quite ugly, try the following:
fx = ((fx > epsilon) ? powf(fx, (1.0f / 3.0f))
: (7.787f * fx + 16.0f / 116.0f));
> +#define LABA_OP(dst, X, op, Y) dst = (LABA) { \
> + (X).l op(Y).l, \
> + (X).A op(Y).A, \
> + (X).b op(Y).b, \
> + (X).a op(Y).a } \
> +
> +#define LABA_OPC(dst, X, op, Y) dst = (LABA) { \
> + (X).l op(Y), \
> + (X).A op(Y), \
> + (X).b op(Y), \
> + (X).a op(Y) } \
> +
> +#define LABA_OP1(dst, op, Y) dst = (LABA) { \
> + dst.l op(Y).l, \
> + dst.A op(Y).A, \
> + dst.b op(Y).b, \
> + dst.a op(Y).a } \
Please align the \.
> +static void square_row(LABA *row, int width)
> +{
> + for (int i = 0; i < width; i++)
Some compilers won't like that C99 variable declaration.
> +/*
> + * Blurs image horizontally (width 2*size+1) and writes it transposed to dst
> + * (called twice gives 2d blur)
> + * Callback is executed on every row before blurring
Switch from third person singular to impersonal form and end the
sentences in periods.
> +/*
> + * Filters image with callback and blurs (lousy approximate of gaussian)
see above
> + * it proportionally to
proportionally to what?
> + // Compose image on coloured background to better judge dissimilarity
> with various backgrounds
Break the line
> + if (n & 4) {
> + f1.l += 1.0 - f1.a; // using premultiplied alpha
> + }
> + if (n & 8) {
> + f1.A += 1.0 - f1.a;
> + }
> + if (n & 16) {
> + f1.b += 1.0 - f1.a;
> + }
Drop the {}.
> +/*
> + * Algorithm based on Rabah Mehdi's C++ implementation
> + *
> + * frees memory in read_info structs.
> + * saves dissimilarity visualisation as ssimfilename (pass NULL if not
> needed)
I don't see anything about a filename in the function
> +#define SSIM(r) \
> + ((2.0 * (mu1[k].r * mu2[k].r) + c1) * \
> + (2.0 * (sigma12[k].r - (mu1[k].r * mu2[k].r)) + c2)) / \
> + (((mu1[k].r * mu1[k].r) + (mu2[k].r * mu2[k].r) + c1) * \
> + ((sigma1_sq[k].r - (mu1[k].r * mu1[k].r)) + \
> + (sigma2_sq[k].r - (mu2[k].r * mu2[k].r)) + c2))
Align the \.
> + r = (1.0 - ssim.a) + maxsq;
pointless ()
> --- /dev/null
> +++ b/libavfilter/vf_compare.c
> @@ -0,0 +1,278 @@
> + ff_formats_ref(formats, &ctx->inputs [MAIN ]->out_formats);
> + ff_formats_ref(formats, &ctx->inputs [REF ]->out_formats);
> + ff_formats_ref(formats, &ctx->outputs[MAIN ]->in_formats);
spaces inside [], ugh ..
> +AVFilter avfilter_vf_compare = {
> + .name = "compare",
> + .description = NULL_IF_CONFIG_SMALL("Compare two video sources"),
> +
> + .init = init,
> + .uninit = uninit,
> +
> + .priv_size = sizeof(CmpContext),
> +
> + .query_formats = query_formats,
> +
> + .inputs = vf_compare_inputs,
> + .outputs = vf_compare_outputs,
vertical align
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
