[FFmpeg-devel] [PATCH] mov.c: factorize ff_mov_read_stsd_entries()

[Michael Niedermayer]

On Sat, Mar 14, 2009 at 11:47:08AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, Mar 13, 2009 at 10:36 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> > On Fri, Mar 13, 2009 at 10:28 PM, Baptiste Coudurier
> > <baptiste.coudurier at gmail.com> wrote:
> >> Humm. multiple stsd, or really multiple palette for one stsd ?
> >
> > I think it's all under the same stsd, just multiple pseudo-streams
> > inside it. I don't get mov at all.
> 
> attached is an ugly proof-of-concept patch that sort-of allows
> decoding of the MSRLE content. It's ugly, probably breaks any other
> file format containing MSRLE, etc.etc., but it sort-of shows how to do
> this. Some frames still don't decode correctly (compared to
> quicktime), but it looks better than before (where every single frame
> was wrong).
> 
> Based on this patch, I'd like some feedback on how to implement this
> correctly (my patch is clearly wrong).
> 
> Ronald

> Index: ffmpeg-svn/libavcodec/msrle.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/msrle.c	2009-03-14 11:44:09.000000000 -0400
> +++ ffmpeg-svn/libavcodec/msrle.c	2009-03-14 11:44:26.000000000 -0400
> @@ -78,13 +78,14 @@
>      }
>  
>      /* make the palette available */
> -    memcpy(s->frame.data[1], s->avctx->palctrl->palette, AVPALETTE_SIZE);
> -    if (s->avctx->palctrl->palette_changed) {
> -        s->frame.palette_has_changed = 1;
> -        s->avctx->palctrl->palette_changed = 0;
> -    }
> +    //memcpy(s->frame.data[1], s->avctx->palctrl->palette, AVPALETTE_SIZE);
> +    //if (s->avctx->palctrl->palette_changed) {
> +    //    s->frame.palette_has_changed = 1;
> +    //    s->avctx->palctrl->palette_changed = 0;
> +    //}
> +    memcpy(s->frame.data[1], buf, AVPALETTE_SIZE);
>  
> -    ff_msrle_decode(avctx, (AVPicture*)&s->frame, avctx->bits_per_coded_sample, buf, buf_size);
> +    ff_msrle_decode(avctx, (AVPicture*)&s->frame, avctx->bits_per_coded_sample, buf + AVPALETTE_SIZE, buf_size - AVPALETTE_SIZE);
>  
>      *data_size = sizeof(AVFrame);
>      *(AVFrame*)data = s->frame;

you cant do this, data[1] is native endian, the codec bitstream must not
depend on the cpu endianness

also you redundantly store the palette in each packet.
Also any change to the codec bitstream format must be combined with changes
to all (de)muxers that support that codec.

what you can do is,
store the string "Palette" at AVPacket.data[0]
then 1 byte indicating which first palette entry changed, and then a byte
indicating how many are updated (0=256)
and store 4 byte RGBA for each entry changed.

the decoder can then check for "Palette" and extract it if one is there.
The rest of the packet would be the normal frame.

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20090314/59f223e9/attachment.pgp>