[FFmpeg-devel] [PATCH v6] lavf: palettized QuickTime video in Matroska

[Michael Niedermayer]

On Wed, Dec 23, 2015 at 03:37:18PM +0100, Mats Peterson wrote:
> I mistakenly used 'extradata' rather than 'st->codec->extradata',
> naturally leading to a segfault. Here's an updated patch.
> 
> An explanation of the patch follows:
> 
> Palettized QuickTime video in Matroska has hitherto not been
> recognized whatsoever, and the "palette" used has been completely
> random.
> 
> The patch for matroskadec.c fixes this issue by adding a palette
> side data packet in matroska_deliver_packet(), much in the same way
> as it's done in mov.c.
> 
> The change to mov.c consists mainly of moving the palette handling
> from the mov_parse_stsd_video() function to a new ff_get_qtpalette()
> function in the new file qtpalette.c, which is shared by both
> matroskadec.c and mov.c.
> 
> In matroskadec.c, I'm also putting the palette in 'extradata', like
> it's done for V_MS/VFW/FOURCC; this is a requirement in order for
> MPlayer to recognize the palette. And why is this, you may wonder.
> Well, it's because for some mysterious reason, MPlayer adds ANOTHER
> palette side data packet after the one added in matroskadec.c. It
> uses whatever is in extradata as the palette when adding this
> packet.
> 
> Mats
> 
> -- 
> Mats Peterson
> http://matsp888.no-ip.org/~mats/

>  Makefile      |    1 
>  matroskadec.c |   30 +++++++++++++-
>  mov.c         |   92 +++++++++----------------------------------
>  qtpalette.c   |  123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qtpalette.h   |    6 ++
>  5 files changed, 178 insertions(+), 74 deletions(-)
> 46ebbf197c0580b7588cb4550373a05489454cae  0001-lavf-palettized-QuickTime-video-in-Matroska.patch
> From 91376a9e4e0cb4a7522be565e248616c4803c0ad Mon Sep 17 00:00:00 2001
> From: Mats Peterson <matsp888 at yahoo.com>
> Date: Wed, 23 Dec 2015 15:30:15 +0100
> Subject: [PATCH v6] lavf: palettized QuickTime video in Matroska
[...]
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 06e80c8..5b0c5af 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c

> @@ -1751,13 +1751,20 @@ static int mov_codec_id(AVStream *st, uint32_t format)
>      return id;
>  }
>  
> -static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb,
> +static int mov_parse_stsd_video(MOVContext *c, AVIOContext *pb,
>                                   AVStream *st, MOVStreamContext *sc)
>  {
> +    uint8_t *stsd;
>      uint8_t codec_name[32];
> -    unsigned int color_depth, len, j;
> -    int color_greyscale;
> -    int color_table_id;
> +    int64_t pos;
> +    unsigned int len;
> +
> +    if (!(stsd = av_malloc(70)))
> +        return AVERROR(ENOMEM);

the malloc is unneeded, an array on the stack could be used (its just
a fixed 70 bytes)
this would also simplify the error handling

[...]

> @@ -310,4 +311,7 @@ static const uint8_t ff_qt_default_palette_256[256 * 3] = {
>    /* 255, 0xFF */  0x00, 0x00, 0x00
>  };
>  
> +int ff_get_qtpalette(int codec_id, uint8_t *stsd, AVIOContext *pb,
> +        uint32_t *palette);
> +

missing doxy documentation, missing "const" for unchanged arrays
also why does this need a "byte" array and a AVIOContext as input
arguments ?
iam asking as this looks a bit confusing with 2 inputs ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151227/9c60646d/attachment.sig>