[FFmpeg-devel] [PATCH] Remove swscale_internal.h:fmt_depth()

Stefano Sabatini stefano.sabatini-lala
Mon Jan 18 02:04:51 CET 2010


On date Sunday 2010-01-17 22:25:22 -0200, Ramiro Polla encoded:
> On Sun, Jan 17, 2010 at 10:17 PM, Stefano Sabatini
> <stefano.sabatini-lala at poste.it> wrote:
> > On date Sunday 2010-01-17 23:36:26 +0100, Stefano Sabatini encoded:
> >> On date Sunday 2010-01-17 22:04:47 +0100, Michael Niedermayer encoded:
> >> > On Sun, Jan 17, 2010 at 06:46:40PM +0100, Stefano Sabatini wrote:
> >> > > On date Saturday 2010-01-16 17:08:48 -0200, Ramiro Polla encoded:
> >> > > > Hi,
> >> > > >
> >> > > > On Sat, Jan 16, 2010 at 4:59 PM, Stefano Sabatini
> >> > > > <stefano.sabatini-lala at poste.it> wrote:
> >> > > > > Hi, I'm aware this patch introduces a slow-down, an idea would be to
> >> > > > > initialize a ff_bits_per_pixel array during the init phase, and then
> >> > > > > use a function of the kind:
> >> > > > >
> >> > > > > static inline int fmt_depth(int fmt)
> >> > > > > {
> >> > > > > ? ?return ff_bits_per_pixel[fmt];
> >> > > > > }
> >> > > > >
> >> > > > > Would be that acceptable?
> >> > > > > In this case can you suggest where to initialize stuff?
> >> > > >
> >> > > > I think all code that uses fmt_depth currently should eventually be
> >> > > > moved to some init code that's only run once, and so a small slow-down
> >> > > > wouldn't be a problem.
> >> > >
> >> > > Check the attached: smaller, more extensible, faster, the price is a
> >> > > little more bloat in the context.
> >> > >
> >> >
> >> > > Regression test passed.
> >> >
> >> > if(regression == swscale_example) patch ok
> >> > else not ok
> >>
> >> I had to hack swscale-example since the recent change in pixfmt.h
> >> broke it (BTW does it ever worked with big-endian system?), anyway
> >> what should I test with swscale-example?
> >>
> >> I tried to run swscale-example first and before the patch, and then
> >> many times with the same binary, each time I got some small differences
> >> in the outputs, for example (two runs with the same binary):
> >>
> >> stefano at geppetto ~/s/f/libswscale> diff -u swscale-example2.out swscale-example3.out
> >> --- swscale-example2.out ? ? ?2010-01-17 23:23:35.000000000 +0100
> >> +++ swscale-example3.out ? ? ?2010-01-17 23:29:15.000000000 +0100
> >> @@ -50514,12 +50514,12 @@
> >> ? argb 96x96 -> rgba ? 96x ?64 flags= 8 SSD= ? ?6, ? ?0, ? ?0, ? ?1
> >> ? argb 96x96 -> rgba ? 96x ?64 flags=16 SSD= ? 12, ? ?0, ? ?0, ? 16
> >> ? argb 96x96 -> rgba ? 96x ?64 flags=32 SSD= ? ?9, ? ?1, ? ?2, ? ?1
> >> - argb 96x96 -> rgba ? 96x ?96 flags= 1 SSD= 1729, 1202, ?729, 4513
> >> - argb 96x96 -> rgba ? 96x ?96 flags= 2 SSD= 1729, 1180, ?720, 4513
> >> - argb 96x96 -> rgba ? 96x ?96 flags= 4 SSD= 1729, 1202, ?730, 4513
> >> - argb 96x96 -> rgba ? 96x ?96 flags= 8 SSD= 1729, 1190, ?724, 4513
> >> - argb 96x96 -> rgba ? 96x ?96 flags=16 SSD= 1729, 1234, ?755, 4513
> >> - argb 96x96 -> rgba ? 96x ?96 flags=32 SSD= 1729, 1202, ?729, 4513
> >> + argb 96x96 -> rgba ? 96x ?96 flags= 1 SSD= 1728, 1202, ?729, 4513
> >> + argb 96x96 -> rgba ? 96x ?96 flags= 2 SSD= 1728, 1180, ?720, 4513
> >> + argb 96x96 -> rgba ? 96x ?96 flags= 4 SSD= 1728, 1202, ?729, 4513
> >> + argb 96x96 -> rgba ? 96x ?96 flags= 8 SSD= 1728, 1189, ?724, 4513
> >> + argb 96x96 -> rgba ? 96x ?96 flags=16 SSD= 1728, 1234, ?755, 4513
> >> + argb 96x96 -> rgba ? 96x ?96 flags=32 SSD= 1728, 1202, ?729, 4513
> >> ? argb 96x96 -> rgba ? 96x 128 flags= 1 SSD= ? ?8, ? ?1, ? ?2, ? ?0
> >> ? argb 96x96 -> rgba ? 96x 128 flags= 2 SSD= ? ?8, ? ?1, ? ?1, ? ?0
> >> ? argb 96x96 -> rgba ? 96x 128 flags= 4 SSD= ? ?5, ? ?0, ? ?0, ? ?0
> >> @@ -55282,12 +55282,12 @@
> >> ? abgr 96x96 -> bgra ? 96x ?64 flags= 8 SSD= ? ?6, ? ?0, ? ?0, ? ?1
> >> ? abgr 96x96 -> bgra ? 96x ?64 flags=16 SSD= ? 12, ? ?0, ? ?0, ? 16
> >> ? abgr 96x96 -> bgra ? 96x ?64 flags=32 SSD= ? ?9, ? ?1, ? ?2, ? ?1
> >> - abgr 96x96 -> bgra ? 96x ?96 flags= 1 SSD= 1729, 1196, ?724, 4513
> >> - abgr 96x96 -> bgra ? 96x ?96 flags= 2 SSD= 1729, 1174, ?716, 4513
> >> - abgr 96x96 -> bgra ? 96x ?96 flags= 4 SSD= 1729, 1197, ?725, 4513
> >> - abgr 96x96 -> bgra ? 96x ?96 flags= 8 SSD= 1729, 1183, ?719, 4513
> >> - abgr 96x96 -> bgra ? 96x ?96 flags=16 SSD= 1729, 1229, ?751, 4513
> >> - abgr 96x96 -> bgra ? 96x ?96 flags=32 SSD= 1729, 1196, ?724, 4513
> >> + abgr 96x96 -> bgra ? 96x ?96 flags= 1 SSD= 1728, 1196, ?724, 4513
> >> + abgr 96x96 -> bgra ? 96x ?96 flags= 2 SSD= 1728, 1174, ?716, 4513
> >> + abgr 96x96 -> bgra ? 96x ?96 flags= 4 SSD= 1728, 1197, ?725, 4513
> >> + abgr 96x96 -> bgra ? 96x ?96 flags= 8 SSD= 1728, 1183, ?719, 4513
> >> + abgr 96x96 -> bgra ? 96x ?96 flags=16 SSD= 1728, 1229, ?751, 4513
> >> + abgr 96x96 -> bgra ? 96x ?96 flags=32 SSD= 1728, 1196, ?724, 4513
> >> ? abgr 96x96 -> bgra ? 96x 128 flags= 1 SSD= ? ?8, ? ?1, ? ?2, ? ?0
> >> ? abgr 96x96 -> bgra ? 96x 128 flags= 2 SSD= ? ?8, ? ?1, ? ?1, ? ?0
> >> ? abgr 96x96 -> bgra ? 96x 128 flags= 4 SSD= ? ?5, ? ?0, ? ?0, ? ?0
> >>
> >> which I found very strange and still cannot explain (where is the
> >> random source, lfg in swscale-example.c uses always the same seed).
> >
> > argb -> bgra
> > abgr -> bgra
> >
> > both are not supported, so the scaler doesn't write nothing in output
> > and this explains the high SSD value and the randomicity of the
> > output.
> >
> > Time to fix rgb2rgbWrapper!
> 
> I think it should be:
> diff --git a/swscale.c b/swscale.c
> index a2a8c83..0f6d631 100644
> --- a/swscale.c
> +++ b/swscale.c
> @@ -2550,8 +2550,8 @@ SwsContext *sws_getContext(int srcW, int srcH,
> enum PixelFormat srcFormat, int d
>             && srcFormat != PIX_FMT_RGB4_BYTE && dstFormat != PIX_FMT_RGB4_BYTE
>             && srcFormat != PIX_FMT_MONOBLACK && dstFormat != PIX_FMT_MONOBLACK
>             && srcFormat != PIX_FMT_MONOWHITE && dstFormat != PIX_FMT_MONOWHITE
> -                                             && dstFormat != PIX_FMT_RGB32_1
> -                                             && dstFormat != PIX_FMT_BGR32_1
> +           && srcFormat != PIX_FMT_RGB32_1   && dstFormat != PIX_FMT_RGB32_1
> +           && srcFormat != PIX_FMT_BGR32_1   && dstFormat != PIX_FMT_BGR32_1
>             && srcFormat != PIX_FMT_RGB48LE   && dstFormat != PIX_FMT_RGB48LE
>             && srcFormat != PIX_FMT_RGB48BE   && dstFormat != PIX_FMT_RGB48BE
>             && (!needsDither || (c->flags&(SWS_FAST_BILINEAR|SWS_POINT))))

This could be simplified checking for:
!{src,dst}FormatDesc.flags & PIX_FMT_BITSTREAM 

> 
> But I didn't test yet because swscale-example doesn't build because of
> the latest change in pixfmt.h.

(I simply re-added the _NE definitions which were in pixfmt.h, clearly
not the right solution).

Patch attached fix the SSD for R/G/B, the alpha SSD is still high,
which depends on the hack in r27522 which can't work with alpha, maybe
we need some more specialized converters.

Regards.
-- 
FFmpeg = Faboulous and Faboulous Mastodontic Programmable Elaborated Geek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-argb-rgba-conversion.patch
Type: text/x-diff
Size: 1727 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100118/f1a9bcdc/attachment.patch>



More information about the ffmpeg-devel mailing list