[MPlayer-dev-eng] [PATCH] Correctly support 32-bit raw MOV
Michael Niedermayer
michaelni at gmx.at
Fri Jan 25 02:47:38 CET 2008
On Wed, Jan 23, 2008 at 06:28:40PM +0800, Timothy Lee wrote:
> Dear all,
>
> Michael Niedermayer wrote:
>> On Tue, Jan 22, 2008 at 02:51:42PM +0800, Timothy Lee wrote:
>>
>>> Carl Eugen Hoyos wrote:
>>>
>>>> Timothy Lee<timothy.lee<at> siriushk.com> writes:
>>>>
>>>>> The following patches fixes the support for 32-bit raw RGB video in
>>>>> MOV. The colorspace of those video are IMGFMT_RGB32_1, which is not
>>>>> supported by libswscale (as mentioned in
>>>>> http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2007-
>>>> Would it be possible to add a scaler to libswscale to support
>>>> IMGFMT_RGB32_1? I
>>>> believe that solution would be preferred.
>>>>
>>> I've tried down that path, but it appears to involve adding converter
>>> functions for every output format,
>>>
>> no, it would be possible to add just the bswap code and a
>> rgb32_1 -> internal yuv converter
>> one could even add just the bswap32 filter and nothing else, that would be
>> ugly as many convertions would fail but it still is better than this hack
>> id say the bswap32 filter is rejected. ive no comment about the rest
>>
> I've re-implemented the support for RGB32x and BGR32x colorspace inside
> libswscale as per Michael's suggestions. This updated set of patches does
> the following:
>
> * libswscale-rgb32x.patch -- adds support for RGB32x -> BGR32 and
> BGR32x -> RGB32 conversion using 32-bit byte-swapping.
> * vd-rgb32x.patch -- modifies the filter chain construction process
> to automatically insert another vf_scale when the vo does not
> support RGB32x or BGR32x colorspace.
> * mov-raw-32bit.patch -- fixes the colorspace of 32-bit raw RGB
> video reported by MOV demuxer.
>
> Again, the code has not been tested on big-endian machines.
>
> Regards,
> Timothy Lee
> Index: fmt-conversion.h
> ===================================================================
> --- fmt-conversion.h (revision 25830)
> +++ fmt-conversion.h (working copy)
> @@ -11,6 +11,8 @@
> enum PixelFormat imgfmt2pixfmt(int fmt)
> {
> switch (fmt) {
> + case IMGFMT_BGR32|64:
> + return PIX_FMT_RGB32_1;
> case IMGFMT_BGR32:
> return PIX_FMT_RGB32;
> case IMGFMT_BGR24:
> @@ -30,6 +32,8 @@
> return PIX_FMT_RGB4_BYTE;
> case IMGFMT_BG4B:
> return PIX_FMT_BGR4_BYTE;
> + case IMGFMT_RGB32|64:
> + return PIX_FMT_BGR32_1;
> case IMGFMT_RGB32:
> return PIX_FMT_BGR32;
> case IMGFMT_RGB24:
IMGFMT_*|64 is ugly, use only named formats dont write code with assumtions
about specific numeric values of IMGFMT_*
[...]
>
> /*
> - supported Input formats: YV12, I420/IYUV, YUY2, UYVY, BGR32, BGR24, BGR16, BGR15, RGB32, RGB24, Y8/Y800, YVU9/IF09, PAL8
> + supported Input formats: YV12, I420/IYUV, YUY2, UYVY, BGR32, BGR24, BGR16, BGR15, RGB32, RGB24, Y8/Y800, YVU9/IF09, PAL8, RGB32x, BGR32x
the formats are not called RGB32x anywhere
[...]
> +/* Swap bytes to convert RGB32x -> BGR32, or BGR32x -> RGB32 */
> +static int bswap32Wrapper(SwsContext *c, uint8_t* src[], int srcStride[],
not doxygen compatible
> + int srcSliceY, int srcSliceH, uint8_t* dst[], int dstStride[])
> +{
> + long i = 0;
> + uint32_t* s = (uint32_t*)(src[0]);
> + uint32_t* d = (uint32_t*)(dst[0] + dstStride[0] * srcSliceY);
> + for (i = (srcStride[0] * srcSliceH) >> 2; i >= 0; i--, s++, d++)
> + *d = bswap_32(*s);
> +}
srcStride[0] can be negative, also this is messy, remove all ',' from the for()
please
> +
> /* unscaled copy like stuff (assumes nearly identical formats) */
> static int simpleCopy(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
> int srcSliceH, uint8_t* dst[], int dstStride[]){
[...]
> + /* rgb32x -> bgr32, or bgr32x -> rgb32 */
> + if (((srcFormat == PIX_FMT_RGB32_1) && (dstFormat == PIX_FMT_BGR32)) ||
> + ((srcFormat == PIX_FMT_BGR32_1) && (dstFormat == PIX_FMT_RGB32)))
> + c->swScale= bswap32Wrapper;
these are only 2 of the 4 convertions for which bswap32Wrapper can be used
also please look at:
static inline void RENAME(rgb32ToY)(uint8_t *dst, uint8_t *src, int width)
static inline void RENAME(rgb32ToUV)(uint8_t *dstU, uint8_t *dstV, uint8_t *src1, uint8_t *src2, int width)
and implement the matching funtions for the new formats
its a mere copy and paste with minor changes for the different RGB ordering
> +
> /* rgb/bgr -> rgb/bgr (no dither needed forms) */
> if ( (isBGR(srcFormat) || isRGB(srcFormat))
> && (isBGR(dstFormat) || isRGB(dstFormat))
> Index: libmpcodecs/vd.c
> ===================================================================
> --- libmpcodecs/vd.c (revision 25830)
> +++ libmpcodecs/vd.c (working copy)
> @@ -130,10 +130,12 @@
> int mpcodecs_config_vo(sh_video_t *sh, int w, int h, unsigned int preferred_outfmt){
> int i,j;
> unsigned int out_fmt=0;
> + unsigned int bswap_fmt=0;
> int screen_size_x=0;//SCREEN_SIZE_X;
> int screen_size_y=0;//SCREEN_SIZE_Y;
> vf_instance_t* vf=sh->vfilter,*sc=NULL;
> int palette=0;
> + int bswap=0;
> int vocfg_flags=0;
>
> if(w)
> @@ -178,8 +180,28 @@
> mp_msg(MSGT_CPLAYER,MSGL_DBG2,"vo_debug: codec query_format(%s) returned FALSE\n",vo_format_name(out_fmt));
> continue;
> }
> + bswap = 0;
> j=i; vo_flags=flags; if(flags&VFCAP_CSP_SUPPORTED_BY_HW) break;
> } else
> + if ((out_fmt == (IMGFMT_RGB32|64) || out_fmt == (IMGFMT_BGR32|64))){
> + // Check normalized form of RGB32x and BGR32x format
> + unsigned int alt_fmt = 0;
> + if (out_fmt == (IMGFMT_RGB32|64)) alt_fmt = IMGFMT_BGR32;
> + if (out_fmt == (IMGFMT_BGR32|64)) alt_fmt = IMGFMT_RGB32;
> + flags = vf->query_format(vf, alt_fmt);
> + mp_msg(MSGT_CPLAYER,MSGL_DBG2,"vo_debug: query(%s) returned 0x%X (i=%d) \n",
> + vo_format_name(alt_fmt), flags, i);
> + if((flags&VFCAP_CSP_SUPPORTED_BY_HW) || (flags&VFCAP_CSP_SUPPORTED && j<0)){
> + // check (query) if codec really support this outfmt...
> + sh->outfmtidx=j; // pass index to the control() function this way
> + if(mpvdec->control(sh,VDCTRL_QUERY_FORMAT,&out_fmt)==CONTROL_FALSE){
> + mp_msg(MSGT_CPLAYER,MSGL_DBG2,"vo_debug: codec query_format(%s) returned FALSE\n",vo_format_name(out_fmt));
> + continue;
> + }
> + bswap = 1; bswap_fmt = out_fmt;
> + j=i; vo_flags=flags; if(flags&VFCAP_CSP_SUPPORTED_BY_HW) break;
> + }
> + } else
> if(!palette && !(flags&(VFCAP_CSP_SUPPORTED_BY_HW|VFCAP_CSP_SUPPORTED)) && (out_fmt==IMGFMT_RGB8||out_fmt==IMGFMT_BGR8)){
> sh->outfmtidx=j; // pass index to the control() function this way
> if(mpvdec->control(sh,VDCTRL_QUERY_FORMAT,&out_fmt)!=CONTROL_FALSE)
> @@ -221,6 +244,11 @@
> sh->vf_inited=-1;
> return 0; // failed
> }
> + if (bswap) {
> + // Insert filter to normalize RGB32x/BGR32x if necessary
> + vf = vf_open_filter(vf, "scale", NULL);
> + vf->query_format(vf, bswap_fmt);
> + }
> out_fmt=sh->codec->outfmt[j];
> mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_UsingXAsOutputCspNoY,vo_format_name(out_fmt),j);
> sh->outfmtidx=j;
i dont know the code but iam 99% sure its 99% wrong and unneeded
> Index: libmpcodecs/vf_scale.c
> ===================================================================
> --- libmpcodecs/vf_scale.c (revision 25830)
> +++ libmpcodecs/vf_scale.c (working copy)
> @@ -85,16 +85,47 @@
> 0
> };
>
> -static unsigned int find_best_out(vf_instance_t *vf){
> +// Supported output format for IMGFMT_RGB32_1
> +static unsigned int outfmt_rgb32x_list[]={
> + IMGFMT_BGR32,
> + 0
> +};
> +
> +// Supported output format for IMGFMT_BGR32_1
> +static unsigned int outfmt_bgr32x_list[]={
> + IMGFMT_RGB32,
> + 0
> +};
> +
> +static unsigned int find_best_out(vf_instance_t *vf, unsigned int fmt){
> unsigned int best=0;
> int i;
>
> + // Choose the list of possible output format depending on input format
> + const unsigned int* list;
> + int max_i;
> + if (fmt == (IMGFMT_RGB32|64))
> + {
> + list = outfmt_rgb32x_list;
> + max_i = sizeof(outfmt_rgb32x_list) / sizeof(int) - 1;
> + }
> + else if (fmt == (IMGFMT_BGR32|64))
> + {
> + list = outfmt_bgr32x_list;
> + max_i = sizeof(outfmt_bgr32x_list) / sizeof(int) - 1;
> + }
> + else
> + {
> + list = outfmt_list;
> + max_i = sizeof(outfmt_list) / sizeof(int) - 1;
> + }
uhm, you are adding some scary hacks to avoid writing 10 lines of code in
swscale.c
[...]
> case IMGFMT_RG4B:
> + case IMGFMT_BGR32|64:
> + case IMGFMT_RGB32|64:
> {
again ONLY straight IMGFMT_ macros are acceptable to be used outside img_format.*!
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker, user
questions for the command line tools ffmpeg, ffplay, ... as well as questions
about how to use libav* should be sent to the ffmpeg-user mailinglist.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20080125/6b230b5b/attachment.pgp>
More information about the MPlayer-dev-eng
mailing list