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

[Baptiste Coudurier]

On Wed, Apr 15, 2009 at 09:52:27PM +0200, Reimar D?ffinger wrote:
> On Wed, Apr 15, 2009 at 12:21:57PM -0700, Baptiste Coudurier wrote:
> > On Wed, Apr 15, 2009 at 09:00:36PM +0200, Reimar D?ffinger wrote:
> > > [...]
> > >
> > > I don't really consider it worth the effort.
> > 
> > I really consider it worth the effort.
> 
> Then please go ahead.

Sure, I will.

> > I understand the problem with race condition is because the demuxer
> > actually updates AVCodecContext palette directly, but IMHO that can be
> > fixed by letting application dealing with palette change.
> 
> So your solution is just to remove the only way to pass the palette that is
> there?

Absolutely not.

> Sorry, I really don't see the point of continuing discussion if you only provide
> solutions that are so much simpler because they don't even half-work and in
> some other cases because you are free to choose whichever way is more convenient
> for them problem but exclude each other (e.g. "compressed" vs. uncompressed
> palette data).

I don't agree with you. I propose alternatives but you just don't want
to consider them. It's not what I call try to find compromises nor consensus.

There are some way to avoid av_grow_packet. I proposed alternatives. Now
please let's try to find a compromise.

IMHO it's not fair nor justified to ignore Ronald's and my comments.

> It's just another bikeshed like that and I'm really tired of the
> endless discussions it causes with in the end nobody doing any fix whatsoever,
> just like in the DV demuxer case.

Well IMHO adding another demuxer is poluting allcodecs/makefiles
and is not needed.

But don't worry I will eventually fix it myself ;)

> > If formats didn't judge pertinent to store the palette within the frame,
> > why are we obsessed in doing this ?
> 
> _I_ am not talking about AVI, for the other formats they can be just as much
> considered "within" the frame as they could be considered separate IMO.
> 
> > Well, your patch assumes that libavcodec is used unfortunately,
> 
> No.

Yes it does. 

> > and that libs are updated at the same time too.
> 
> As would any change removing AVPaletteControl

Well I proposed an alternative which would not remove AVPaletteControl.

> 
> > If we don't decode palette in demuxer, we could pass raw data to
> > decoder in some way. 
> 
> For example in the way my patch does
> 
> > I just don't like appending palette in packet data because this forces
> > parsing.
> 
> Again, _I_ am not considering AVI/ASF and I make no claim as to what the
> proper solution for them is, but _my_ patch mostly only _moves_ code from
> the demuxer to the decoder.

Well code still appends (compressed) palette to packet data.

As a side note, I don't think av_grow_packet, av_append_packet belongs to
public API, nor av_shrink_packet but it's too late.

Also WC3 case might use extradata to store palette this would solve
the mov/avi/mkv case at first.

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