On Thu, Nov 13, 2014 at 09:39:49PM +0900, TOYAMA Shin-ichi wrote:
> Michael Niedermayer wrote in <20141111172026.GS32286@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 <sh...@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}.
>
> +@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.
pgpmhlnexer3R.pgp
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
