[FFmpeg-devel] [PATCH] DVBSUBTILES, fixes to decoding/encoding and a new way of burning the subtitles onto the screen

[Clément Bœsch]

On Tue, Sep 27, 2011 at 08:24:24PM +0100, JULIAN GARDNER wrote:
[...]
> >>  Changes made to dvbsub*.c are to fix to the encoding and decoding, not real 
> > much to say but if you use the old source against the TS file i posted to the 
> > bug system months ago you will see that it did not work, it does now.
> > 
> > i tried with ticket/153/bbc_small.ts
> > playing it with ffplay works, but not pretty, the colors are off but
> > it works. applyimg your patch makes it worse, there are artifacts
> > at the right and left of the first "help" text and valgrind reports
> > below
> > 
> > ==31934== Use of uninitialised value of size 8
> > ==31934==    at 0x43B0EB: video_image_display (ffplay.c:533)
> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> > ==31934==
> > ==31934== Use of uninitialised value of size 8
> > ==31934==    at 0x43B136: video_image_display (ffplay.c:539)
> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> > ==31934==
> > ==31934== Use of uninitialised value of size 8
> > ==31934==    at 0x43B17E: video_image_display (ffplay.c:547)
> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> > ==31934==
> > ==31934== Use of uninitialised value of size 8
> > ==31934==    at 0x43B205: video_image_display (ffplay.c:553)
> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> > ==31934==
> > 
> 
> Attached is a new patch against my previous code which fixes the problem
> above, what is happening is that we have corrupted data and i was not
> waiting for a correct start of data. Patch also removes some old code
> and replaces it with calls to av_log_dump.
> 

Why do you insist on mixing a lot of changes? :)

> Hopefully this is usuable, i used 'git diff', if its no good let me know.
> 

You can use commit only the chunks you want to submit with `git add -p`,
use git format-patch HEAD^ to generate the patch, and send it.

[...]
> diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
> index a846dbf..3745682 100644
> --- a/libavcodec/dvbsubdec.c
> +++ b/libavcodec/dvbsubdec.c
> @@ -236,6 +236,7 @@ typedef struct DVBSubContext {
>  
>      int version;
>      int time_out;
> +    int start_epoch;
>      DVBSubRegion *region_list;
>      DVBSubCLUT   *clut_list;
>      DVBSubObject *object_list;
> @@ -322,7 +323,6 @@ static void delete_region_display_list(DVBSubContext *ctx, DVBSubRegion *region)
>  
>          av_free(display);
>      }
> -

Please do not mix such changes in your patches.

>  }
>  
>  static void delete_cluts(DVBSubContext *ctx)
> @@ -593,7 +593,6 @@ static int dvbsub_read_4bit_string(uint8_t *destbuf, int dbuf_len,
>  
>  #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) {
> @@ -791,20 +790,8 @@ static void dvbsub_parse_pixel_data_block(AVCodecContext *avctx, DVBSubObjectDis
>      uint8_t *map_table;
>  
>  #if 0
> -    av_dlog(avctx, "DVB pixel block size %d, %s field:\n", buf_size,
> -            top_bottom ? "bottom" : "top");
> -
> -    for (i = 0; i < buf_size; i++) {
> -        if (i % 16 == 0)
> -            av_dlog(avctx, "0x%8p: ", buf+i);
> -
> -        av_dlog(avctx, "%02x ", buf[i]);
> -        if (i % 16 == 15)
> -            av_dlog(avctx, "\n");
> -    }
> -
> -    if (i % 16)
> -        av_dlog(avctx, "\n");
> +    av_log( avctx, AV_LOG_INFO, "DVB pixel block size\n");

We do not put spaces after '(' when no vertical alignment is required.

> +    av_log_dump( avctx, AV_LOG_INFO, buf, buf_size);
>  #endif
>  
>      if (region == 0)
> @@ -816,9 +803,6 @@ static void dvbsub_parse_pixel_data_block(AVCodecContext *avctx, DVBSubObjectDis
>      x_pos = display->x_pos;
>      y_pos = display->y_pos;
>  
> -//    if ((y_pos & 1) != top_bottom)
> -//        y_pos++;
> -

Avoid such unrelated changes please. Also the debug you added belongs to
another patch.

[...]

>      timeout = *buf++;
>      version = ((*buf)>>4) & 15;
>      page_state = ((*buf++) >> 2) & 3;
> @@ -1213,6 +1203,7 @@ static void dvbsub_parse_page_segment(AVCodecContext *avctx,
>              delete_regions(ctx);
>              delete_objects(ctx);
>              delete_cluts(ctx);
> +            ctx->start_epoch = 1;
>          }
>  
>          tmp_display_list = ctx->display_list;
> @@ -1225,6 +1216,9 @@ static void dvbsub_parse_page_segment(AVCodecContext *avctx,
>              av_free(display);
>          }
>  
> +        if( !ctx->start_epoch)
> +            return;
> +

OK so this if the interesting chunk?… Use "if (", not "if( " for consistency.

>          ctx->display_list = NULL;
>          ctx->display_list_size = 0;
>  
> @@ -1268,7 +1262,7 @@ static void dvbsub_parse_page_segment(AVCodecContext 
>  }

[...]
>  
>      p = buf;
> @@ -1556,7 +1551,6 @@ static int dvbsub_decode(AVCodecContext *avctx,
>      return p - buf;
>  }
>  
> -

Unrelated again… :)


-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110927/3d470d97/attachment.asc>