[FFmpeg-devel] Fwd: [PATCH] lavfi: add xbr filter

[Michael Niedermayer]

Hi

On Sat, Nov 08, 2014 at 08:27:18PM +0530, arwa arif wrote:
> On Sat, Nov 8, 2014 at 6:21 PM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> 
> > On Sat, Nov 08, 2014 at 05:12:13PM +0530, arwa arif wrote:
> > [...]
> >
> > > +static int df(uint32_t x, uint32_t y, const uint32_t *r2y)
> > > +{
> > > +
> > > +#define YMASK 0xff0000
> > > +#define UMASK 0x00ff00
> > > +#define VMASK 0x0000ff
> > > +
> > > +    uint32_t yuv1 = r2y[x & 0xffffff];
> > > +    uint32_t yuv2 = r2y[y & 0xffffff];
> > > +
> > > +    return abs((yuv1 & YMASK) - (yuv2 & YMASK)) > (48 << 16) ||
> > > +           abs((yuv1 & UMASK) - (yuv2 & UMASK)) > ( 7 <<  8) ||
> > > +           abs((yuv1 & VMASK) - (yuv2 & VMASK)) > ( 6 <<  0);
> >
> > the "> some constant" is wrong
> > the new code expects this to be a scalar not a boolean
> > adding the 3 absolute differences together should work better
> >
> > [...]
> >
> >
> > Updated the patch.

seems you lost ffmpeg-devel from the CC
I just CC-ed you directly on the previous mails to make sure you
dont miss the review/reply, normally all discussion is on ffmpeg-dev
without CCes
the patch and review should stay on ffmpeg-devel

more comments below


[...]
> + * Hyllian's 2xBR v3.3b
> + * 
> + * Copyright (C) 2011 Hyllian <sergiogdb at gmail.com>

https://github.com/yoyofr/iFBA/blob/master/fba_src/src/intf/video/scalers/xbr.cpp
contains
"Copyright (C) 2011, 2012 Hyllian/Jararaca - sergiogdb at gmail.com"

so unless i miss something that exact copyright line should be kept
or already asked the author(s) and they agreed to LGPL


> + *
> + * Copyright (c) 2014 Arwa Arif <arwaarif1994 at gmail.com>
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.

https://github.com/yoyofr/iFBA/blob/master/fba_src/src/intf/video/scalers/xbr.cpp
is GPL v2+
ive read elsewhere that Hyllian wanted it to be LGPL, so LGPL should
be fine but we should confirm that all authors of this file actually
agree to LGPL and until then you can simply replace the LGPL header
by the GPL v2+ one from xbr.cpp
that is unless you found and used only code from LGPL licensed xbr


> + *
> + * FFmpeg 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * XBR Filter is used for depixelization of image.
> + * This is based on Hyllian's 2xBR shader.
> + *
> + * @see : http://www.libretro.com/forums/viewtopic.php?f=6&t=134
> + * @see : https://github.com/yoyofr/iFBA/blob/master/fba_src/src/intf/video/scalers/xbr.cpp
> + * Future work : To implement x3 and x4 scale, and threading.
> + */
> +
> +#include "libavutil/opt.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/pixdesc.h"
> +#include "internal.h"
> +
> +#define RGB_MASK         0x00FFFFFF
> +#define pg_lbmask        0x00FEFEFE
> +#define pg_red_blue_mask 0x00FF00FF
> +#define pg_green_mask    0x0000FF00
> +
> +typedef struct {
> +    uint32_t rgbtoyuv[1<<24];
> +} xBRContext;
> +
> +static uint32_t df(uint32_t x, uint32_t y, const uint32_t *r2y)
> +{
> +
> +#define YMASK 0x00ff0000
> +#define UMASK 0x0000ff00
> +#define VMASK 0x000000ff
> +
> +    uint32_t yuv1 = r2y[x & 0x00ffffff];
> +    uint32_t yuv2 = r2y[y & 0x00ffffff];
> +

> +    return abs((yuv1 & YMASK) - (yuv2 & YMASK)) > (48 << 16) +
> +           abs((yuv1 & UMASK) - (yuv2 & UMASK)) > ( 7 <<  8) +
> +           abs((yuv1 & VMASK) - (yuv2 & VMASK)) > ( 6 <<  0);

the "> (some number)" is not correct df() is intended to be a
scalar distance, aka distance between 2 points, like 7 meters would
be. they are like points in  Y,U,V space instead of real world right/left,
up/down, forward/backward.
the sum of the abs() should work without any ">", the first 2 just
need to be scaled down by 16 and 8 bit

i think this is the only technical issue left, the patch should be
fine after that to be applied

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141108/61011bad/attachment.asc>