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

[Michael Niedermayer]

On Wed, Apr 15, 2009 at 12:09:36PM -0700, Baptiste Coudurier wrote:
> On Wed, Apr 15, 2009 at 09:01:38PM +0200, Michael Niedermayer wrote:
> > On Wed, Apr 15, 2009 at 11:49:54AM -0700, Baptiste Coudurier wrote:
> > > On 4/15/2009 11:35 AM, Michael Niedermayer wrote:
> > > > On Wed, Apr 15, 2009 at 10:51:22AM -0700, Baptiste Coudurier wrote:
> > > >> On 4/15/2009 10:33 AM, Michael Niedermayer wrote:
> > > >>> On Wed, Apr 15, 2009 at 10:22:26AM -0700, Baptiste Coudurier wrote:
> > > >>>> Hi,
> > > >>>>
> > > >>>> On 4/11/2009 3:19 PM, Reimar D?ffinger wrote:
> > > >>>>> Hello,
> > > >>>>> attached patch implements this.
> > > >>>>> It is a bit ugly because it adds a lot of seeking to the demuxer, but I
> > > >>>>> think for such a fringe format there is no point in extra work for avoiding it.
> > > >>>>> The patch also adds two helper functions, one to increase the size of a
> > > >>>>> packet and one that appends data read from a file to a possibly already
> > > >>>>> filled packet.
> > > >>>>> I expect that there will be more that a few bugs still, e.g. while the
> > > >>>>> video looks visually ok in ffplay the FATE regressions do not match at
> > > >>>>> all.
> > > >>>> Well it seems that palette could be stored in extradata in it's read in
> > > >>>> wv3_read_header, a way to pass palette change information through
> > > >>>> AVPacket would be needed.
> > > >>>>
> > > >>>> Also AVPacket could have ->palette_data field which seems a lot cleaner
> > > >>>> to me and Ronald.
> > > >>> palette_data would be a nightmare in pretty much every aspect
> > > >>> first many formats allow multiple palette chunks to update a palette
> > > >>> for a frame. Yes AVI too which i had forgotten, it too can update
> > > >>> 2 entries in a palette through 2 pc chunks that each changes just 1
> > > >>> entry.
> > > >> Please enumerate the formats. State exactly what the problem is.
> > > > 
> > > > i think i did
> > > > 
> > > > 
> > > >> Doesn't avi use bmp header ? If so palette -> extradata.
> > > > 
> > > > if you dont belive me you can read the source, look at sample files
> > > > (like CIMOVI01.avi) or the avi spec
> > > > 
> > > > 
> > > >>> second, muxers that do not support such palettes (mov/mp4/asf?/rm/mkv/nut/...)
> > > >>> must somehow merge them and this should really have been done in the demuxer
> > > >>> already.
> > > >> I'm not sure about this, in _any_ case -vcodec copy will cause problems
> > > >> since you have merged the palette and the data in the packet.
> > > > 
> > > > -vcodec copy will not cause problems if muxers and demuxers implement the
> > > > API correctly, this is true for any not to crappy API.
> > > 
> > > Parsing packet data is annoying in any case and IMHO it will only
> > 
> > well the palette chunks need to be parsed as they arent complete palettes
> > you cant avoid that
> 
> Yes you can. If you don't append the palette to packet data you don't
> need to parse the packet.

you dont even need to decode it or display it because without the palette
it wouldnt look too good anyway


[...]
> > > >> If the muxer can handle palette seperatly (mov does) it can create
> > > >> multiple stsd with correct palette information. It avoids parsing
> > > >> palette in the packet which is _really_ ugly.
> > > > 
> > > > the first sample file i picked has
> > > > 87 palette chunks and a duration of 7.8 seconds (at 12fps)
> > > > are you sure that 3600*30 stsd chunks in mov will work?
> > > 
> > > Well it will work for sure. 
> > 
> > how exactly would the libvaformat mov demuxer pass these palettes
> > to the decoder?
> 
> Using the same way it does currently ?

You mean the one that causes race conditions and doesnt work with threaded
players like ffplay ...
The one why we try to change because it doesnt work ...

[...]
> > > [...]
> > > 
> > > 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?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/ffmpeg-devel/attachments/20090415/6a4e2e10/attachment.pgp>