[FFmpeg-devel] [PATCH] read palette information from extradata for dvd subtitle decoder

[Michael Niedermayer]

On Fri, Jan 18, 2008 at 12:42:42AM +0100, elupus wrote:
> Hi,
> 
> Here's a patch to read the dvd palette from extradata in the dvdsub
> decoder, this allows it to read matroska stored subtitles (and external
> vobsubs if player can supply it) with the correct color displayed.
> 
> It relies on extradata being padded, but i think that is alright.
> 
> It introduces a context for this information. Some other stuff could
> probably be moved to it too, but not needed for now.
> 
> Should be simple to add stuff like offsets/custom colors/alignments.
> scaling would be very much harder todo in decoder, but it's probably not
> it's job anyway.
> 
> I suppose the string based extradata isn't the best way to store that
> stuff, but seems silly to invent a new format since matroska and vobsub idx
> files already use this.
> 
> Regards
> Joakim
Content-Description: Attached file: dvdsub.patch
> Index: libavcodec/dvdsubdec.c
> ===================================================================
> --- libavcodec/dvdsubdec.c	(revision 11552)
> +++ libavcodec/dvdsubdec.c	(working copy)
> @@ -25,6 +25,14 @@
>  
>  //#define DEBUG
>  
> +typedef struct DVDSubContext
> +{
> +  uint8_t  colormap[4];
> +  uint8_t  alpha[4];
> +  uint32_t palette[16];
> +  int      has_palette;
> +} DVDSubContext;
> +
>  static void yuv_a_to_rgba(const uint8_t *ycbcr, const uint8_t *alpha, uint32_t *rgba, int num_values)
>  {
>      uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;
> @@ -115,14 +123,22 @@
>      return 0;
>  }
>  
> -static void guess_palette(uint32_t *rgba_palette,
> -                          uint8_t *colormap,
> -                          uint8_t *alpha,
> -                          uint32_t subtitle_color)
> +static void fill_palette(DVDSubContext* ctx,
> +                         uint32_t *rgba_palette,
> +                         uint32_t subtitle_color)
>  {
>      uint8_t color_used[16];
>      int nb_opaque_colors, i, level, j, r, g, b;
> +    uint8_t *colormap = ctx->colormap;
> +    uint8_t *alpha = ctx->alpha;
>  
> +    if(ctx->has_palette) {
> +        for(i = 0; i < 4; i++)
> +            rgba_palette[i] = (ctx->palette[ctx->colormap[i]] & 0x00ffffff)
> +                              | ((alpha[i] * 17) << 24);
> +        return;
> +    }

why dont you just read extradata as it is needed?
you wouldnt need the context and all these cosmetic changes ...


[...]
> +static int dvdsub_init(AVCodecContext *avctx)
> +{
> +    DVDSubContext *ctx = (DVDSubContext*) avctx->priv_data;
> +    char *data = avctx->extradata;
> +
> +    if (!avctx->extradata || !avctx->extradata_size)
> +        return 1;
> +
> +    data[avctx->extradata_size] = '\0';

you cannot write in extradata, and absolutely never over extradata_size


> +
> +    for(;;) {
> +        int pos = strcspn(data, "\n\r");

what is it \n ? \r ? any? both?


> +        if (pos==0 && *data==0)
> +            break;
> +
> +        if (strncmp("palette:", data, 8) == 0) {
> +            int i;
> +            char *p = data+8;
> +            ctx->has_palette = 1;
> +            for(i=0;i<16;i++) {
> +                ctx->palette[i] = strtoul(p, &p, 16);
> +                while(*p == ',' || isspace(*p))
> +                    p++;
> +            }

some validity check to avoid segfaults would be nice


> +#if defined(DEBUG)
> +            av_log(avctx, AV_LOG_WARNING, "palette:");
> +            for(i=0;i<16;i++)
> +                av_log(avctx, AV_LOG_WARNING, " 0x%06x ", ctx->palette[i]);
> +            av_log(avctx, AV_LOG_WARNING, "\n");
> +#endif

remove this


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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker, user
questions for the command line tools ffmpeg, ffplay, ... as well as questions
about how to use libav* should be sent to the ffmpeg-user mailinglist.
-------------- 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/20080124/5ccad552/attachment.pgp>