[FFmpeg-devel] [PATCH] DVBSUBTILES, fixes to decoding/encoding and a new way of burning the subtitles onto the screen
Michael Niedermayer
michaelni at gmx.at
Fri Oct 14 20:33:56 CEST 2011
On Sun, Sep 25, 2011 at 12:18:20AM +0200, Clément Bœsch wrote:
> From: joolzg <joolzg at btinternet.com>
>
> ---
> Here is the second patch from Julian Gardner.
>
> I did my best to remove the unrelated changes, fix the style and various other
> things, but it's still far from perfect. Some real reviews are needed.
>
> This patch includes various changes that might be splitted and explained. Also,
> there is a pretty big hack in ffmpeg.c that should be fixed.
ive split and applied some of the changes to dvbsubdec.
comments for the rest of dvbsubdec are below
[...]
> @@ -462,16 +471,18 @@ static av_cold int dvbsub_close_decoder(AVCodecContext *avctx)
>
> static int dvbsub_read_2bit_string(uint8_t *destbuf, int dbuf_len,
> const uint8_t **srcbuf, int buf_size,
> - int non_mod, uint8_t *map_table)
> + int non_mod, uint8_t *map_table, int x_pos)
> {
> GetBitContext gb;
>
> int bits;
> int run_length;
> - int pixels_read = 0;
> + int pixels_read = x_pos;
>
> init_get_bits(&gb, *srcbuf, buf_size << 3);
>
> + destbuf += x_pos;
> +
> while (get_bits_count(&gb) < buf_size << 3 && pixels_read < dbuf_len) {
> bits = get_bits(&gb, 2);
>
This change and similar ones are unneeded. they just change how xpos
is passed
> @@ -532,14 +543,14 @@ static int dvbsub_read_2bit_string(uint8_t *destbuf, int dbuf_len,
> }
> }
> } else if (bits == 1) {
> - pixels_read += 2;
> if (map_table)
> bits = map_table[0];
> else
> bits = 0;
> - if (pixels_read <= dbuf_len) {
> - *destbuf++ = bits;
> + run_length = 2;
> + while (run_length-- > 0 && pixels_read < dbuf_len) {
> *destbuf++ = bits;
> + pixels_read++;
> }
> } else {
> (*srcbuf) += (get_bits_count(&gb) + 7) >> 3;
I assume this is a bugfix thus applied
> @@ -567,16 +578,23 @@ static int dvbsub_read_2bit_string(uint8_t *destbuf, int dbuf_len,
>
> static int dvbsub_read_4bit_string(uint8_t *destbuf, int dbuf_len,
> const uint8_t **srcbuf, int buf_size,
> - int non_mod, uint8_t *map_table)
> + int non_mod, uint8_t *map_table, int x_pos)
> {
> GetBitContext gb;
>
> int bits;
> int run_length;
> - int pixels_read = 0;
> + int pixels_read = x_pos;
>
> init_get_bits(&gb, *srcbuf, buf_size << 3);
>
> + destbuf += x_pos;
> +
> +#ifdef DEBUG
> + av_log(NULL, AV_LOG_WARNING, " in:dvbsub_read_4bit_string xpos:%d bufSize:%d pointer:%p\n", x_pos, dbuf_len, destbuf);
> + av_log(NULL, AV_LOG_WARNING, " in:dvbsub_read_4bit_string xpos:%d bufSize:%d\n", x_pos, dbuf_len);
> +#endif
> +
> while (get_bits_count(&gb) < buf_size << 3 && pixels_read < dbuf_len) {
> bits = get_bits(&gb, 4);
>
> @@ -595,6 +613,9 @@ static int dvbsub_read_4bit_string(uint8_t *destbuf, int dbuf_len,
>
> if (run_length == 0) {
> (*srcbuf) += (get_bits_count(&gb) + 7) >> 3;
> +#ifdef DEBUG
> + av_log(NULL, AV_LOG_WARNING, "out:dvbsub_read_4bit_string xpos:%d bufSize:%d\n", pixels_read, dbuf_len);
> +#endif
> return pixels_read;
> }
>
[...]
> @@ -683,17 +704,23 @@ static int dvbsub_read_4bit_string(uint8_t *destbuf, int dbuf_len,
>
> (*srcbuf) += (get_bits_count(&gb) + 7) >> 3;
>
> +#ifdef DEBUG
> + av_log(NULL, AV_LOG_WARNING, "out:dvbsub_read_4bit_string xpos:%d bufSize:%d\n", pixels_read, dbuf_len);
> +#endif
> +
> return pixels_read;
> }
>
> static int dvbsub_read_8bit_string(uint8_t *destbuf, int dbuf_len,
> const uint8_t **srcbuf, int buf_size,
> - int non_mod, uint8_t *map_table)
> + int non_mod, uint8_t *map_table, int x_pos)
> {
> const uint8_t *sbuf_end = (*srcbuf) + buf_size;
> int bits;
> int run_length;
> - int pixels_read = 0;
> + int pixels_read = x_pos;
> +
> + destbuf += x_pos;
>
> while (*srcbuf < sbuf_end && pixels_read < dbuf_len) {
> bits = *(*srcbuf)++;
again, unneeded
> @@ -762,6 +789,7 @@ static void dvbsub_parse_pixel_data_block(AVCodecContext *avctx, DVBSubObjectDis
> 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff};
> uint8_t *map_table;
>
> +#if 0
> av_dlog(avctx, "DVB pixel block size %d, %s field:\n", buf_size,
> top_bottom ? "bottom" : "top");
>
> @@ -776,21 +804,22 @@ static void dvbsub_parse_pixel_data_block(AVCodecContext *avctx, DVBSubObjectDis
>
> if (i % 16)
> av_dlog(avctx, "\n");
> +#endif
>
> if (region == 0)
> return;
>
> pbuf = region->pbuf;
> + region->dirty = 1;
>
> x_pos = display->x_pos;
> y_pos = display->y_pos;
>
> - if ((y_pos & 1) != top_bottom)
> - y_pos++;
> + y_pos += top_bottom;
>
> while (buf < buf_end) {
> - if (x_pos > region->width || y_pos > region->height) {
> - av_log(avctx, AV_LOG_ERROR, "Invalid object location!\n");
> + if (*buf!=0xf0 && (x_pos >= region->width || y_pos >= region->height)) {
> + av_log(avctx, AV_LOG_ERROR, "Invalid object location! %d-%d %d-%d %02x\n", x_pos, region->width, y_pos, region->height, *buf);
> return;
> }
>
applied
> @@ -803,9 +832,9 @@ static void dvbsub_parse_pixel_data_block(AVCodecContext *avctx, DVBSubObjectDis
> else
> map_table = NULL;
>
> - x_pos += dvbsub_read_2bit_string(pbuf + (y_pos * region->width) + x_pos,
> - region->width - x_pos, &buf, buf_end - buf,
> - non_mod, map_table);
> + x_pos = dvbsub_read_2bit_string(pbuf + (y_pos * region->width),
> + region->width, &buf, buf_end - buf,
> + non_mod, map_table, x_pos);
> break;
> case 0x11:
> if (region->depth < 4) {
> @@ -818,9 +847,9 @@ static void dvbsub_parse_pixel_data_block(AVCodecContext *avctx, DVBSubObjectDis
> else
> map_table = NULL;
>
> - x_pos += dvbsub_read_4bit_string(pbuf + (y_pos * region->width) + x_pos,
> - region->width - x_pos, &buf, buf_end - buf,
> - non_mod, map_table);
> + x_pos = dvbsub_read_4bit_string(pbuf + (y_pos * region->width),
> + region->width, &buf, buf_end - buf,
> + non_mod, map_table, x_pos);
> break;
> case 0x12:
> if (region->depth < 8) {
> @@ -828,9 +857,9 @@ static void dvbsub_parse_pixel_data_block(AVCodecContext *avctx, DVBSubObjectDis
> return;
> }
>
> - x_pos += dvbsub_read_8bit_string(pbuf + (y_pos * region->width) + x_pos,
> - region->width - x_pos, &buf, buf_end - buf,
> - non_mod, NULL);
> + x_pos = dvbsub_read_8bit_string(pbuf + (y_pos * region->width),
> + region->width, &buf, buf_end - buf,
> + non_mod, NULL, x_pos);
> break;
>
> case 0x20:
unneeded
> @@ -865,7 +894,6 @@ static void dvbsub_parse_object_segment(AVCodecContext *avctx,
> DVBSubContext *ctx = avctx->priv_data;
>
> const uint8_t *buf_end = buf + buf_size;
> - const uint8_t *block;
> int object_id;
> DVBSubObject *object;
> DVBSubObjectDisplay *display;
> @@ -896,7 +924,12 @@ static void dvbsub_parse_object_segment(AVCodecContext *avctx,
> }
>
> for (display = object->display_list; display; display = display->object_list_next) {
> - block = buf;
> + const uint8_t *block = buf;
> + int bfl = bottom_field_len;
> +
> +#ifdef DEBUG
> + av_log(avctx, AV_LOG_WARNING, " Object %d, Display %p, Block %p, Tfl %d, Bfl %d\n", object_id, display, block, top_field_len, bfl);
> +#endif
>
> dvbsub_parse_pixel_data_block(avctx, display, block, top_field_len, 0,
> non_modifying_color);
> @@ -904,9 +937,9 @@ static void dvbsub_parse_object_segment(AVCodecContext *avctx,
> if (bottom_field_len > 0)
> block = buf + top_field_len;
> else
> - bottom_field_len = top_field_len;
> + bfl = top_field_len;
>
> - dvbsub_parse_pixel_data_block(avctx, display, block, bottom_field_len, 1,
> + dvbsub_parse_pixel_data_block(avctx, display, block, bfl, 1,
> non_modifying_color);
> }
>
> @@ -923,8 +956,8 @@ static void dvbsub_parse_clut_segment(AVCodecContext *avctx,
> {
> DVBSubContext *ctx = avctx->priv_data;
>
> - const uint8_t *buf_end = buf + buf_size;
> int i, clut_id;
> + int version;
> DVBSubCLUT *clut;
> int entry_id, depth , full_range;
> int y, cr, cb, alpha;
> @@ -942,7 +975,9 @@ static void dvbsub_parse_clut_segment(AVCodecContext *avctx,
> av_dlog(avctx, "\n");
>
> clut_id = *buf++;
> + version = ((*buf)>>4)&15;
> buf += 1;
> + buf_size -= 2;
>
> clut = get_clut(ctx, clut_id);
>
> @@ -952,28 +987,37 @@ static void dvbsub_parse_clut_segment(AVCodecContext *avctx,
> memcpy(clut, &default_clut, sizeof(DVBSubCLUT));
>
> clut->id = clut_id;
> + clut->version = -1;
>
> clut->next = ctx->clut_list;
> ctx->clut_list = clut;
> }
>
> - while (buf + 4 < buf_end) {
> + if (clut->version != version) {
LGTM thus applied
[...]
> tmp_display_list = ctx->display_list;
> +
> + while (tmp_display_list) {
> + display = tmp_display_list;
> +
> + tmp_display_list = display->next;
> +
> + av_free(display);
> + }
> +
> ctx->display_list = NULL;
> ctx->display_list_size = 0;
>
> - while (buf + 5 < buf_end) {
> + while (buf_size) {
> region_id = *buf++;
> buf += 1;
> + buf_size -= 2;
>
> display = tmp_display_list;
> tmp_ptr = &tmp_display_list;
I do not know what the goal of this code is but
display, tmp_display_list will always be NULL after this and the
following loop (not shown in the patch) will never execute
> @@ -1160,8 +1244,10 @@ static void dvbsub_parse_page_segment(AVCodecContext *avctx,
>
> display->x_pos = AV_RB16(buf);
> buf += 2;
> + buf_size -= 2;
> display->y_pos = AV_RB16(buf);
> buf += 2;
> + buf_size -= 2;
[...]
> rect->x = display->x_pos + offset_x;
> rect->y = display->y_pos + offset_y;
> rect->w = region->width;
> rect->h = region->height;
> - rect->nb_colors = (1 << region->depth);
> rect->type = SUBTITLE_BITMAP;
> rect->pict.linesize[0] = region->width;
>
> clut = get_clut(ctx, region->clut);
> -
> if (!clut)
> clut = &default_clut;
>
> + rect->pict.data[1] = av_mallocz(AVPALETTE_SIZE);
> switch (region->depth) {
> case 2:
> - clut_table = clut->clut4;
> + memcpy(rect->pict.data[1], clut->clut4, 4 * sizeof(uint32_t));
> + rect->nb_colors = 4;
> break;
> case 8:
> - clut_table = clut->clut256;
> + memcpy(rect->pict.data[1], clut->clut256, 256 * sizeof(uint32_t));
> + rect->nb_colors = 256;
> break;
> case 4:
> default:
> - clut_table = clut->clut16;
> + memcpy(rect->pict.data[1], clut->clut16, 16 * sizeof(uint32_t));
> + rect->nb_colors = 16;
> break;
> }
>
> - rect->pict.data[1] = av_mallocz(AVPALETTE_SIZE);
> - memcpy(rect->pict.data[1], clut_table, (1 << region->depth) * sizeof(uint32_t));
unneeded
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111014/f2c1a3d2/attachment.asc>
More information about the ffmpeg-devel
mailing list