[FFmpeg-devel] [RFC] WC3 decoder without AVPaletteControl

[Baptiste Coudurier]

On Thu, Apr 16, 2009 at 01:08:41AM +0200, Michael Niedermayer wrote:
> On Wed, Apr 15, 2009 at 03:39:50PM -0700, Baptiste Coudurier wrote:
> > On Wed, Apr 15, 2009 at 11:55:48PM +0200, Michael Niedermayer wrote:
> [...]
> > 
> > > [...]
> > > > > > [...]
> > > > > > 
> > > > > > IIRC the race condition happens because demuxer sets palette to
> > > > > > AVCodecContext _directly_.
> > > > > > 
> > > > > > Storing it in AVFormatContext should avoid this race, or is there
> > > > > > something I'm forgetting ?
> > > > > 
> > > > > you forget that the palette from AVFormatContext must reach the decoder
> > > > > and what you do is that you would just remove the code that does that
> > > > > that fixes the race condition but it removes palette support pretty much
> > > > 
> > > > I don't forget, application will update AVCodecContext palette
> > > > accordingly, of course. This does not remove palette support at all,
> > > > and avoids changing decoders and fixes the race condition IMHO since
> > > > now AVCodecContext does not get updated by the demuxer but the application.
> > > 
> > > Which application?
> > > 
> > 
> > One you don't know about ? ;)
> 
> Yes, exactly it will not work with any application i do know about
> ffplay, ffmpeg, vlc, mplayer, xine, ...

Well since fixing AVPalette took several years. I would have thought
player applications to implement the correct solution that is copying
palette from AVCodecContext when palette_change is set.

> > This is still one solution.
> > 
> > There are at least 3 solutions:
> > - append palette to pkt->data
> 
> yes, thats what i suggested
> 
> 
> > - use pkt->palette_data
> 
> this is the same as using pkt->data, it needs grow and append and parsing
> read what reimar wrote
> and it does not work with most muxers as is

Well I don't see how, sorry.

> > - use AVFormatContext palette
> 
> This is equivalent to what we do currently as i said already

Well messing with AVCodecContext when avformat is supposed to handle
this can seen as awkward I agree.

> > 
> > If the third is not ok for anybody then fine.
> > 
> > I find the second reasonable and should satisfy everybody IMHO.
> > Palette is passed through AVPacket and it avoids parsing in pkt->data,
> > though it grows a bit AVPacket struct. It's not like we are afraid of
> > growing structs usually ;)
> 
> you really arent reading what others write
> 
> let me repeat
> formats store several compressed palette chunks that update the previous
> these need merging at the demuxer and parsing at the decoder no matter if
> they are in data or palette_data

Yes, but storing this merged palette in pkt->palette_data instead of
pkt->data appears simpler to Ronald and me. 

I'll repeat what I just said:

if pkt->palette_data then extract_palette in decoder

if pkt->data then damn is there a palette in here ? How do I know ?

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA    
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org