[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