AW: Re: Re: [Ffmpeg-devel] swscale and palette ... what am I missing?

[Karl H. Beckers]

-- Urspr?ngl. Mitteil. --
Message: 1
Date: Thu, 1 Mar 2007 20:10:32 +0100
From: Michael Niedermayer <michaelni at gmx.at>
Subject: Re: Re: [Ffmpeg-devel] swscale and palette ... what am I
	missing?
To: FFmpeg development discussions and patches
	<ffmpeg-devel at mplayerhq.hu>
Message-ID: <20070301191031.GY25795 at MichaelsNB>
Content-Type: text/plain; charset="us-ascii"

Hi

On Wed, Feb 28, 2007 at 09:56:38AM +0000, Karl H. Beckers wrote:
> Am Mittwoch, den 28.02.2007, 01:00 +0100 schrieb
> ffmpeg-devel-request at mplayerhq.hu:
> > [...] 
> > > Am I missing anything obvious?
> > 
> > hmm dunno, very quickly looking at your mail it looks all ok, i
> > suggest you
> > place a few random av_log() in the code to see where the non zero pal
> > becomes
> > NULL
> > 
> > and of course a patch to fix this is very welcome if its a bug :)
> > 
> > [...]
> 
> Well then,
> 
> looking at swscale_template.c seems to suggest the following (though I
> wonder how this can ever have worked in any test, but be that as it
> may):
> 
> static int RENAME(swScale)(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
>               int srcSliceH, uint8_t* dst[], int dstStride[]){
> 
> does not get the pal parameter, but sets it for consecutive functions.
> It does it here:
> 
> if(isPacked(c->srcFormat)){
> 	pal= src[1];
> 	src[0]=
> 	src[1]=
> 	src[2]= src[0];
> 	srcStride[0]=
> 	srcStride[1]=
> 	srcStride[2]= srcStride[0];
> }
> 
> since pal ist set to NULL before that, pal will never be anything else
> if we don't enter here. This raises two issues:
> 
> 1) isPacked did non return true for PIX_FMT_PAL8

my tests where with mplayer which used _RGB8/BGR8 instead of PAL8 i think :)

and patch looks ok

[...]

> 
> 2) the picture passed as input to sws_scale is changed.
> 
> That is smth. you at least need to know. I was keeping the AVFrame
> structure and only loading new data into data[0] and setting data[1]
> only on the first frame, since the palette will stay the same. Because
> the above part of swScale changes data[1] on the source AVFrame to equal
> data[0] in the first call, the palette will be rubbish on the second
> call. But pal will still be valid as long as isPacked returns true. 
> 
> Is this essential, or couldn't we just leave the source AVFrame alone?

hmm are you sure the input is changed? it should be a copy of the fields
which is changed




well, rather sure, though I may have missed smth with the way the function names are transformed to find the right implementation for sws_scale.
The way I understand this is the RENAME(swScale) stuff I was quoting is the implementation that is called directly from my call to sws_scale with the params I pass there. Therefore src[] is passed as a pointer to AVFrame.data[]. Then, if you modify src[1] you are actually modifying the pointer stored in the original AVFrame.data[1]. Anything else would involve creating src as a new 4-element array of some pointer type and copying the elements over from the input. That I cannot find.
Also, simply fixing the pointer in AVFrame.data[1] before each call to sws_scale fixes the issue for me.

Karl