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

[Baptiste Coudurier]

On Wed, Apr 15, 2009 at 11:12:04PM +0200, Reimar D?ffinger wrote:
> On Wed, Apr 15, 2009 at 01:22:10PM -0700, Baptiste Coudurier wrote:
> > > > 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.
> 
> Then how?

Well stricly speaking it's not removing anything.
My solution is extending using AVFormatContext to export the palette
and letting application updating AVCodecContext accordingly.

Or using pkt->palette_data and using avcodec_decode_video2.
This solution is a compromise IMHO.

> > > 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.
> 
> Because to me they look flawed front to end and are not even yet finished enough
> to be code.

I don't think so.
 
> > There are some way to avoid av_grow_packet. I proposed alternatives. Now
> > please let's try to find a compromise.
> 
> I asked I think now three or 4 times for alternatives to av_grow_packet that
> allows assembling a single packet from different locations in a file.
> I quoted one piece of code that could make good use of it and I doubt it
> will be the only one.
> I also suggest a way to do av_append_packet without av_grow_packet, but nobody
> responded either.

I believe pkt->palette_data will avoid av_append and av_grow.

> > IMHO it's not fair nor justified to ignore Ronald's and my comments.
> 
> Given above I can't see you doing any better, and I have been writing actual
> code to solve a problem that has been ignored for months (years?), has already
> the maintainers ok and was ignored for days.

Well I'm waiting for a consensus to be reached here. After this, of
course I can start coding.

> > > 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.
> 
> I sent _two_ different solutions, It still did not get anywhere.

Well sorry then, if you have one solution fixing the problem without
using another demuxer, I think it should be fine. Is it really the
case ?

> > > > Well, your patch assumes that libavcodec is used unfortunately,
> > > 
> > > No.
> > 
> > Yes it does. 
> 
> No it doesn't. Shall we go on like that some longer or are you going to
> start to explain your ideas in a way that makes it possible for others to follow?

If it wasn't assuming this, why do you need to change the decoder at
all ? How can you change the bitstream exported by the demuxer without
breaking API ?

Making it working without changing the decoder would be a proof that
the demuxer is not assuming that libavcodec will be used.

Someone could have workarounded the race condition by using separate
AVCodecContext for demuxer and decoder for example, or using its own
demuxer. Unlikely the case, but it might be possible.

> > > > 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.
> 
> I can't see how it should work without changing applications though.
> I admit I have not considered how mixing lav* versions would work with it.

Changing applications is needed because of avcodec_decode_video2 anyway.

> The decision was that libav* should only use public functions from other libav*,
> in that case there is no choice.

av_get_packet is in libavformat only. grow and shrink could be also.

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