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

[Michael Niedermayer]

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


> produces clutter like the AVMedatata language crap, code has proven it.

i see no problem in AVMedatata


> 
> >> 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?


> It's certainly not efficient.

yes,
3600*30*(256*4 + stsd size) vs 3600*30*16
that is 105mb wasted space per hour and id guess also 105mb of
ram needed for demuxing & muxing, iam certain that will fail on some
embeded hardware, even if just by "out of memory"


> 
> >>> third its another field that bloats small AVPackets, another field that
> >>> must be handled explicitlyin every user application that does not use
> >>> AVPackets natively
> >> And modifying demuxer require updating every decoder.
> > 
> > so would they if it was in AVPacket->palette
> 
> Well, it seems that storing palette in AVFormatContext and let the
> application updating AVCodecContext won't need decoders to be changed.

no, it just needs applications to have their demuxer-decoder interface
rewritten to pass frames + palettes
Even for us (ffplay) that would not be fun. Its probably the worst of all
suggestions ...


> 
> >> AVPacket has a useless ->priv flag already it seems. Adding a field is
> >> _not_ a problem.
> >>
> > 
> >> There is still the possibility to put in in AVFormatContext.
> > 
> > no that is not, we try to fix the race condition that is caused by
> > seperating the palette data from packets
> > 
> 
> 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

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/163f929e/attachment.pgp>