[FFmpeg-devel] Fwd: [PATCH] lavfi: add xbr filter
Michael Niedermayer
michaelni at gmx.at
Sat Nov 8 18:39:55 CET 2014
On Sat, Nov 08, 2014 at 10:29:45PM +0530, arwa arif wrote:
> On Sat, Nov 8, 2014 at 8:56 PM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
>
> > 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
> >
> > [...]
> >
> > Updated the patch.
>
>
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > I have often repented speaking, but never of holding my tongue.
> > -- Xenocrates
> >
> doc/filters.texi | 5
> libavfilter/Makefile | 1
> libavfilter/allfilters.c | 1
> libavfilter/vf_xbr.c | 378 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 385 insertions(+)
> dbc5dcad5a06639df2a0afbf0779ed4a7aef96ab 0001-PATCH-lavfi-add-xbr-filter.patch
> From c80af9c57d1750f1d3b1892e1bc0ee46b0e205ea Mon Sep 17 00:00:00 2001
> From: Arwa Arif <arwaarif1994 at gmail.com>
> Date: Thu, 30 Oct 2014 22:06:20 +0530
> Subject: [PATCH] [PATCH]lavfi: add xbr filter xBR
>
> ---
> doc/filters.texi | 5 +
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/vf_xbr.c | 378 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 385 insertions(+)
> create mode 100644 libavfilter/vf_xbr.c
until the GPL vs LGPL is clarified the filter should be added
to configures list of things that are under GPL so it will
require --enable-gpl
similar to
'blackframe_filter_deps="gpl"'
[...]
> +static uint32_t 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)) << 16) +
> + (abs((yuv1 & UMASK) - (yuv2 & UMASK)) << 8) +
the shifts should be the other direction around ">>"
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- 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/548deef7/attachment.asc>
More information about the ffmpeg-devel
mailing list