On Sat, Nov 08, 2014 at 11:30:44PM +0530, arwa arif wrote:
> On Sat, Nov 8, 2014 at 11:09 PM, Michael Niedermayer <michae...@gmx.at>
> wrote:
>
> > On Sat, Nov 08, 2014 at 10:29:45PM +0530, arwa arif wrote:
> > > On Sat, Nov 8, 2014 at 8:56 PM, Michael Niedermayer <michae...@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 <
> > michae...@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 <sergio...@gmail.com>
> > > >
> > > >
> > > >
> > https://github.com/yoyofr/iFBA/blob/master/fba_src/src/intf/video/scalers/xbr.cpp
> > > > contains
> > > > "Copyright (C) 2011, 2012 Hyllian/Jararaca - sergio...@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 <arwaarif1...@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 <arwaarif1...@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"
> >
> doc/filters.texi | 5
> libavfilter/Makefile | 1
> libavfilter/allfilters.c | 1
> libavfilter/vf_xbr.c | 378
> +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 385 insertions(+)
a
xbr_filter_deps="gpl"
should be added to configure
[...]
> +static void xbr2x(AVFrame * input, AVFrame * output, const uint32_t * r2y)
> +{
> + unsigned int e, i,px;
> + unsigned int ex, ex2, ex3;
> + unsigned int ke, ki;
> +
> + int nextOutputLine = output->linesize[0]>>2;
> +
> + for (int y = 0; y < input->height; y++) {
some old compilers have problems with this syntax
moving the int y to the other declarations avoids this
[...]
> + } else {
> + C1 = sa0[3];
> + PC = sa1[3];
> + PF = sa2[3];
> + PI = sa3[3];
> + I5 = sa4[3];
> +
trailing whitespace
our git hooks prevent trailing whitespace from being pushed
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
