[FFmpeg-devel] [PATCH] Enables parsing DVD color palette directly from IFO file

[Clément Bœsch]

On Thu, Nov 13, 2014 at 09:39:49PM +0900, TOYAMA Shin-ichi wrote:
> Michael Niedermayer wrote in <20141111172026.GS32286 at nb4>
> >> +    if ((in = fopen(p, "r")) == NULL) {
> >
> >why does this not use avio_open2() ?
> >this would permit to also use ifo files in other locations than
> >local files
> 
> Thank you for your suggestion.
> I revised the patch to use avio_open2().
> 

> From ab7d45ce3149fb046dc2df4cd27b808bbb6c9f63 Mon Sep 17 00:00:00 2001
> From: Shin-ichi Toyama <shin1 at wmail.plala.or.jp>
> Date: Thu, 13 Nov 2014 21:29:03 +0900
> Subject: [PATCH] New option for obtaining global palette from .IFO file
> 
> ---
>  doc/decoders.texi      |  3 +++
>  libavcodec/dvdsubdec.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/doc/decoders.texi b/doc/decoders.texi
> index ae20cea..02ff475 100644
> --- a/doc/decoders.texi
> +++ b/doc/decoders.texi
> @@ -191,6 +191,9 @@ numbers (without 0x prefix) separated by comas, for example @code{0d00ee,
>  ee450d, 101010, eaeaea, 0ce60b, ec14ed, ebff0b, 0d617a, 7b7b7b, d1d1d1,
>  7b2a0e, 0d950c, 0f007b, cf0dec, cfa80c, 7c127b}.
>  
> + at item use_ifo_palette
> +Specify the the IFO file from which the global palette is obtained.
> +

nit: "use" suggest a boolean value. -ifo_palette will be better.

Also, while I do not object to the patch, a proper dvdread support would
be welcome.

>  @item forced_subs_only
>  Only decode subtitle entries marked as forced. Some titles have forced
>  and non-forced subtitles in the same track. Setting this flag to @code{1}
> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> index bb28d9e..782c449 100644
> --- a/libavcodec/dvdsubdec.c
> +++ b/libavcodec/dvdsubdec.c
> @@ -28,12 +28,15 @@
>  #include "libavutil/opt.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/avstring.h"
> +#include "libavutil/bswap.h"
> +#include "libavformat/avio.h"
>  
>  typedef struct DVDSubContext
>  {
>    AVClass *class;
>    uint32_t palette[16];
>    char    *palette_str;
> +  char    *ifo_str;
>    int      has_palette;
>    uint8_t  colormap[4];
>    uint8_t  alpha[256];
> @@ -583,6 +586,53 @@ static void parse_palette(DVDSubContext *ctx, char *p)
>      }
>  }
>  
> +static void parse_ifo_palette(DVDSubContext *ctx, char *p)
> +{

return int

> +    AVIOContext *ifo = NULL;
> +    char ifostr[12];
> +    uint32_t sp_pgci, pgci, off_pgc, pgc;
> +    uint8_t yuv[16][4], r, g, b;
> +    int i, y, cb, cr;
> +    int r_add, g_add, b_add;
> +    const uint8_t *cm = ff_crop_tab + MAX_NEG_CROP;
> +
> +    ctx->has_palette = 0;
> +    if (avio_open2(&ifo, p, AVIO_FLAG_READ, NULL, NULL) < 0) {

ret = avio_open2(...);
if (ret < 0) {
    av_log(...)
    return;
}

> +        av_log(NULL, AV_LOG_WARNING, "[dvdsubdec.c] Failed to open IFO file \"%s\"\n", p);

use "ctx" as first argument for av_log, and drop "[dvdsubdec.c]". Same for
the following ones.

> +        return;
> +    }
> +    avio_read(ifo, ifostr, 12);
> +    if (strncmp (ifostr, "DVDVIDEO-VTS", 12) != 0) {

if (avio_read(..., 12) != 12 || memcmp(...)) sounds more secure

> +        av_log(NULL, AV_LOG_WARNING, "[dvdsubdec.c] \"%s\" is not a proper IFO file\n", p);
> +        return;

This leaks ifo context. Do something like:
    ret = AVERROR_INVALIDDATA;
    goto end;

> +    }
> +    avio_seek(ifo, 0xCC, SEEK_SET);
> +    if (avio_read(ifo, (char *)&sp_pgci, 4) == 4) {
> +        pgci = av_be2ne32(sp_pgci) * 2048;
> +        avio_seek(ifo, pgci + 0x0C, SEEK_SET);
> +        if (avio_read(ifo, (char *)&off_pgc, 4) == 4) {
> +            pgc = pgci + av_be2ne32(off_pgc);
> +            avio_seek(ifo, pgc + 0xA4, SEEK_SET);

> +            if ((avio_read(ifo, (unsigned char *) yuv, sizeof(yuv))) == sizeof(yuv)) {

I'm not really comfortable with this cast. Could you flatten yuv instead?

> +                for(i=0; i<16; i++) {
> +                    y  = yuv[i][1];
> +                    cr = yuv[i][2];
> +                    cb = yuv[i][3];
> +                    YUV_TO_RGB1_CCIR(cb, cr);
> +                    YUV_TO_RGB2_CCIR(r, g, b, y);
> +                    ctx->palette[i] = (r << 16) + (g << 8) + b;
> +                }
> +                ctx->has_palette = 1;
> +            }
> +        }
> +    }
> +    if (ctx->has_palette == 0) {

> +        av_log(NULL, AV_LOG_WARNING, "[dvdsubdec.c] Failed to read palette from IFO file \"%s\"\n", p);
> +    }

> +    avio_close(ifo);
> +    return;

end:
    avio_close(ifo);
    return ret;

> +}
> +
>  static int dvdsub_parse_extradata(AVCodecContext *avctx)
>  {
>      DVDSubContext *ctx = (DVDSubContext*) avctx->priv_data;
> @@ -631,6 +681,8 @@ static av_cold int dvdsub_init(AVCodecContext *avctx)
>      if ((ret = dvdsub_parse_extradata(avctx)) < 0)
>          return ret;
>  
> +    if (ctx->ifo_str)
> +        parse_ifo_palette(ctx, ctx->ifo_str);
>      if (ctx->palette_str)
>          parse_palette(ctx, ctx->palette_str);
>      if (ctx->has_palette) {
> @@ -656,6 +708,7 @@ static av_cold int dvdsub_close(AVCodecContext *avctx)
>  #define SD AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_DECODING_PARAM
>  static const AVOption options[] = {
>      { "palette", "set the global palette", OFFSET(palette_str), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD },
> +    { "use_ifo_palette", "obtain the global palette from .IFO file", OFFSET(ifo_str), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD },
>      { "forced_subs_only", "Only show forced subtitles", OFFSET(forced_subs_only), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, SD},
>      { NULL }

Since you're adding an option, please bump micro in libavcodec/version.h

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141113/adfe7392/attachment.asc>