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

[Michael Niedermayer]

On Fri, Nov 07, 2014 at 11:08:54PM +0530, arwa arif wrote:
> On Fri, Nov 7, 2014 at 10:53 PM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> 
> > On Tue, Nov 04, 2014 at 12:26:27AM +0530, arwa arif wrote:
> > > Okay. I would try to complete it by tomorrow.
> >
> > any news about this ?
> > i know this isnt trivial to update to the other reference ...
> >
> >
> > >
> > > On Mon, Nov 3, 2014 at 8:02 PM, Stefano Sabatini <stefasab at gmail.com>
> > wrote:
> > >
> > > > On date Monday 2014-11-03 12:09:50 +0530, arwa arif encoded:
> > [...]
> > > > So we have:
> > > > C++, 2xBR v3.3b, LGPL
> > > >
> > > >
> > http://sourceforge.net/p/paintown/code/HEAD/tree/trunk/src/util/graphics/sdl/xbr.cpp
> > > >
> > > > C++, 2xBR v3.3b, GPL
> > > >
> > > >
> > https://github.com/yoyofr/iFBA/blob/master/fba_src/src/intf/video/scalers/xbr.cpp
> > > >
> > > > The former is clearly a copy&paste from the latter, so that should be
> > > > used as reference.
> > > >
> > > > paintown code was relicensed to GPL with Hyllian's consent, so we
> > > > should be able to do the same (if licensing is a concerns, which
> > > > probably isn't for such a fringe filter).
> > > >
> > > > Algorithm description can be found here:
> > > >
> > > >
> > https://web.archive.org/web/20140904180543/http://board.byuu.org/viewtopic.php?f=10&t=2248
> > > >
> > > > (note: it was down the last time I checked).
> > > >
> > > > NOTE: since both are games/emulators, I suppose it won't be easy to
> > > > make them generate a reference output, so we should rely on comparing
> > > > the output generated by the filter with some output found on the web.
> > > >
> > > > @arwa: are you willing to readapt your xbr filter to make use of
> > > > Hyllian's code?
> > > > --
> > > > FFmpeg = Freak and Frightening Maxi Powered Enhancing Gangster
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> >
> >
> I tried modifying the code. I have created a patch, but I am confused how
> to do ALPHA_BLEND part without using float values. Currently I have
> commented that part in the patch. In the reference code(
> https://github.com/yoyofr/iFBA/blob/master/fba_src/src/intf/video/scalers/xbr.cpp),
> they are actually doing ALPHA_BLEND with masks for 565 format. But, since
> FFmpeg has RGB24 format, I am not able to get a way which would do the same
> thing without float.

you do not need floats for the blend stuff
these macros simply mix 2 pixels and the reference does it using
"bit tricks", which is something quite cool

the 128 aka 50:50 mixing works simply by taking the 2 16bit values
RRRRRGGGGGBBBBB and the second pixel rrrrrgggggbbbbb
zeros the least significant bits of each
RRRR0GGGG0BBBB0 and the second pixel rrrr0gggg0bbbb0
and then adds the 2 together as if they where one value

the other blend variants work by spliting green out so there are
unused and zeroed bits between the components
RRRRR00000BBBBB and 00000GGGGG00000 / rrrrr00000bbbbb and 00000ggggg00000
the other operations are carefully selected to scale the red and blue
values without them affecting each other or the green

this stuff above is very interresting and sometimes quite usefull to
perform multiple computations at once but you should not need to
understand this for xbr at least not deeply because i suspect
the original macros work fine with updated bitmasks for
rgb24/rgb32/bgr24/bgr32

#define pg_lbmask        0xFEFEFE
#define pg_red_blue_mask 0xFF00FF
#define pg_green_mask    0x00FF00


[...]
> +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;
> +    unsigned int ts, td;
> +
> +    int nextOutputLine = output->linesize[0];

this is wrong, nextOutputLine is used as an index into a uint32_t *
while linesize is byte based, it thus needs a /4

also the implementation works with RGB/BGR32 but query_formats()
asks for AV_PIX_FMT_RGB24, AV_PIX_FMT_BGR24
this should instead be AV_PIX_FMT_RGB32, AV_PIX_FMT_BGR32
to match the /4 for linesize and the uint32_t
otherwise ifAV_PIX_FMT_RGB24, AV_PIX_FMT_BGR24 is left  its more
tricky


> +
> +    for (int y = 0; y < input->height; y++) {
> +
> +        uint32_t pprev;
> +        uint32_t pprev2;
> +
> +        uint32_t * E = (uint32_t *)(output->data[0] + y * output->linesize[0] * 2);
> +
> +        /* middle. the -2 just makes the offsets later on work out */

> +        uint32_t * sa2 = (uint32_t *)(input->data[0] + y * input->linesize[0] - 2);

the - 2 is similarly wrong as input->data[0] is byte and not pixel
based, it should be a 4*2


> +        /* up one */
> +        uint32_t * sa1 = sa2 - input->linesize[0];
> +        /* up two */
> +        uint32_t * sa0 = sa1 - input->linesize[0];
> +        /* down one */
> +        uint32_t * sa3 = sa2 + input->linesize[0];
> +        /* down two */
> +        uint32_t * sa4 = sa3 + input->linesize[0];

these are wrong the other way around and need /4


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20141107/d9e55040/attachment.asc>