[Ffmpeg-devel] Removing img_format dependency from swscale

[Michael Niedermayer]

Hi

On Wed, Aug 30, 2006 at 04:38:48PM +0200, Luca Abeni wrote:
> Hi all,
> 
> On Tue, 2006-08-29 at 15:17 +0200, Luca Abeni wrote:
> [...]
> > 1) since libswscale is now using ffmpeg's PIX_FMT_* instead of mplayer's
> > IMGFMT_*, swscale.c::sws_orderYUV() cannot use the pixel format to
> > decide if the pointers must be swapped or not.
> Ok, I actually had a look at mplayer's code, and... It seems to me that
> sws_orderYUV() is never called!
> 
> I might be wrong, but here is what I found:
> - Most of the swscale users (libmpcodecs/vf_screenshot.c,
> libmpcodecs/vf_scale.c, libvo/vo_vesa.c, libvo/vo_aa.c, and
> libvo/vo_x11.c) call sws_scale_ordered(), which assume that src[] and
> dst[] are already ordered, and does not call sws_orderYUV()
> - The remaining users (libmpcodecs/vf_sab.c, libmpcodecs/vf_smartblur.c,
> and spudec.c) use the IMGFMT_Y8 (aka PIX_FMT_GRAY8) format, which does
> not need to swap pointers.
> 
> 
> So, it seems to me that I was thinking about a non-issue... :)

hmm, yes seems so, i faintly remember arpi saying something like that the
plane pointers where always ordered in mplayer


> 
> I attach a preliminary version of the patch, for testing. I just tried
> ./mplayer -vo x11 <testfile>
> and
> ./mplayer -vo x11 -vf scale=<w>:<h> <testfile>
> and it seems to work ok...

some preliminary review is below

[...]
> Index: libswscale/yuv2rgb.c
> ===================================================================
> --- libswscale/yuv2rgb.c	(revision 19599)
> +++ libswscale/yuv2rgb.c	(working copy)
> @@ -39,7 +39,6 @@
>  #include "rgb2rgb.h"
>  #include "swscale.h"
>  #include "swscale_internal.h"
> -#include "libmpcodecs/img_format.h" //FIXME try to reduce dependency of such stuff
>  
>  #ifdef HAVE_MLIB
>  #include "yuv2rgb_mlib.c"
> @@ -47,6 +46,43 @@
>  
>  #define DITHER1XBPP // only for mmx
>  
> +#define isBGR(x)       ((x)==PIX_FMT_BGR32 || (x)==PIX_FMT_BGR24\
> +			|| (x)==PIX_FMT_BGR565 || (x)==PIX_FMT_BGR555	\
> +			|| (x)==PIX_FMT_BGR8 || (x)==PIX_FMT_BGR4	\
> +			|| (x)==PIX_FMT_MONOBLACK)
> +
> +int fmt_depth(int fmt)
> +{
> +    switch(fmt) {
> +        case PIX_FMT_BGRA:
> +        case PIX_FMT_ABGR:
> +        case PIX_FMT_RGBA:
> +        case PIX_FMT_ARGB:
> +            return 32;
> +        case PIX_FMT_BGR24:
> +        case PIX_FMT_RGB24:
> +            return 24;
> +        case PIX_FMT_BGR565:
> +        case PIX_FMT_RGB565:
> +            return 16;
> +        case PIX_FMT_BGR555:
> +        case PIX_FMT_RGB555:
> +            return 15;
> +        case PIX_FMT_BGR8:
> +        case PIX_FMT_RGB8:
> +            return 8;
> +        case PIX_FMT_BGR4:
> +        case PIX_FMT_RGB4:
> +        case PIX_FMT_BGR4_BYTE:
> +        case PIX_FMT_RGB4_BYTE:
> +            return 4;
> +        case PIX_FMT_MONOBLACK:
> +            return 1;
> +        default:
> +            return 0;
> +    }
> +}

maybe libavcodec/imgconvert.c pix_fmt_info could be extended with this
stuff
currently theres a bits per channel in it but no
bytes per pixel and no "used" bits per pixel


[...]
> +    switch (format) {
> +        case PIX_FMT_YUV420P:
> +            return "yuv420p";
> +        case PIX_FMT_YUYV422:
> +            return "yuyv422";
> +        case PIX_FMT_RGB24:
> +            return "rgb24";
> +        case PIX_FMT_BGR24:
> +            return "bgr24";
> +        case PIX_FMT_YUV422P:
> +            return "yuv422p";
> +        case PIX_FMT_YUV444P:
> +            return "yuv444p";
> +        case PIX_FMT_RGB32:
> +            return "rgb32";
> +        case PIX_FMT_YUV410P:
> +            return "yuv410p";
> +        case PIX_FMT_YUV411P:
> +            return "yuv411p";
> +        case PIX_FMT_RGB565:
> +            return "rgb565";
> +	case PIX_FMT_RGB555:

tabs vs. spaces ...


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is