[FFmpeg-devel] [PATCH] Add support for parsing the Display Definition Segment in dvbsubdec.c

Janne Grunau janne-ffmpeg
Mon May 24 23:51:20 CEST 2010


On Mon, May 24, 2010 at 10:44:01PM +0200, Michael Niedermayer wrote:
> On Fri, May 21, 2010 at 01:50:15AM +0200, Janne Grunau wrote:
> > 
> > The only (minor) problem I see is that recalculating the subtitle rect's
> > x/y position makes scaling them up to the full display size harder.
> > As far as I understand the only purpose of this part of the DVB subtitle
> > standard is to center SD subtitles reused for HD streams.
> > So it's valid concern but probably not important enough to clutter the
> > API.
> > 
> > Attached patch doesn't change API and changes only x/y values.
> 
> > @@ -334,6 +346,9 @@ static void delete_state(DVBSubContext *ctx)
> >          av_free(clut);
> >      }
> >  
> > +    if (ctx->display_definition)
> > +        av_free(ctx->display_definition);
> 
> the if() is superflous and av_freep is maybe safer

changed to av_freep

> > +
> >      /* Should already be null */
> >      if (ctx->object_list)
> >          av_log(0, AV_LOG_ERROR, "Memory deallocation error!\n");
> 
> > @@ -1254,10 +1269,49 @@ static void save_display_set(DVBSubContext *ctx)
> >  }
> >  #endif
> >  
> > +static void dvbsub_parse_display_definition_segment(AVCodecContext *avctx,
> > +                                                    const uint8_t *buf,
> > +                                                    int buf_size)
> > +{
> > +    DVBSubContext *ctx = (DVBSubContext*) avctx->priv_data;
> 
> unneeded cast

removed

> > +    DVBSubDisplayDefinition *display_def = ctx->display_definition;
> > +    int dds_version, info_byte;
> > +
> > +    if (buf_size < 5)
> > +        return;
> > +
> > +    info_byte   = bytestream_get_byte(&buf);
> > +    dds_version = (info_byte >> 4) & 0xF;
> 
> have you ever seen a byte that had its upper 4 bits > 0xf ?
> ;)

no, fixed :)

> > +    if (display_def && (display_def->version == dds_version))
> > +        return; // already have this display definition version
> > +
> > +    if (!display_def) {
> > +        ctx->display_definition = av_mallocz(sizeof(DVBSubDisplayDefinition));
> > +        display_def = ctx->display_definition;
> > +    }
> > +
> > +    display_def->version = dds_version;
> > +    display_def->x       = 0;
> > +    display_def->y       = 0;
> > +    display_def->width   = bytestream_get_be16(&buf) + 1;
> > +    display_def->height  = bytestream_get_be16(&buf) + 1;
> > +
> > +    if (buf_size < 13)
> > +        return;
> > +
> > +    if ((info_byte >> 3) & 1) { // display_window_flag
> 
> i would write & (1<<3) but i guess gcc can optimize that itself

changed, I think it is easier to read.

also removed a couple of superfluous parentheses and checks av_mallocz return
values.

Janne
-------------- next part --------------
commit cb6c7ba08b84a7f571f1db3c144be04aecadedde
Author: Janne Grunau <janne at grunau.be>
Date:   Sun May 23 23:53:34 2010 +0200

    dvbsub: parse display segment

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index 54c74b5..a815056 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -22,6 +22,7 @@
 #include "dsputil.h"
 #include "get_bits.h"
 #include "colorspace.h"
+#include "bytestream.h"
 
 //#define DEBUG
 //#define DEBUG_PACKET_CONTENTS
@@ -31,6 +32,7 @@
 #define DVBSUB_REGION_SEGMENT   0x11
 #define DVBSUB_CLUT_SEGMENT     0x12
 #define DVBSUB_OBJECT_SEGMENT   0x13
+#define DVBSUB_DISPLAYDEFINITION_SEGMENT 0x14
 #define DVBSUB_DISPLAY_SEGMENT  0x80
 
 #define cm (ff_cropTbl + MAX_NEG_CROP)
@@ -216,6 +218,15 @@ typedef struct DVBSubRegion {
     struct DVBSubRegion *next;
 } DVBSubRegion;
 
+typedef struct DVBSubDisplayDefinition {
+    int version;
+
+    int x;
+    int y;
+    int width;
+    int height;
+} DVBSubDisplayDefinition;
+
 typedef struct DVBSubContext {
     int composition_id;
     int ancillary_id;
@@ -227,6 +238,7 @@ typedef struct DVBSubContext {
 
     int display_list_size;
     DVBSubRegionDisplay *display_list;
+    DVBSubDisplayDefinition *display_definition;
 } DVBSubContext;
 
 
@@ -334,6 +346,8 @@ static void delete_state(DVBSubContext *ctx)
         av_free(clut);
     }
 
+    av_freep(&ctx->display_definition);
+
     /* Should already be null */
     if (ctx->object_list)
         av_log(0, AV_LOG_ERROR, "Memory deallocation error!\n");
@@ -1254,10 +1268,51 @@ static void save_display_set(DVBSubContext *ctx)
 }
 #endif
 
+static void dvbsub_parse_display_definition_segment(AVCodecContext *avctx,
+                                                    const uint8_t *buf,
+                                                    int buf_size)
+{
+    DVBSubContext *ctx = avctx->priv_data;
+    DVBSubDisplayDefinition *display_def = ctx->display_definition;
+    int dds_version, info_byte;
+
+    if (buf_size < 5)
+        return;
+
+    info_byte   = bytestream_get_byte(&buf);
+    dds_version = info_byte >> 4;
+    if (display_def && display_def->version == dds_version)
+        return; // already have this display definition version
+
+    if (!display_def) {
+        display_def             = av_mallocz(sizeof(*display_def));
+        ctx->display_definition = display_def;
+    }
+    if (!display_def)
+        return;
+
+    display_def->version = dds_version;
+    display_def->x       = 0;
+    display_def->y       = 0;
+    display_def->width   = bytestream_get_be16(&buf) + 1;
+    display_def->height  = bytestream_get_be16(&buf) + 1;
+
+    if (buf_size < 13)
+        return;
+
+    if (info_byte & 1<<3) { // display_window_flag
+        display_def->x = bytestream_get_be16(&buf);
+        display_def->y = bytestream_get_be16(&buf);
+        display_def->width  = bytestream_get_be16(&buf) - display_def->x + 1;
+        display_def->height = bytestream_get_be16(&buf) - display_def->y + 1;
+    }
+}
+
 static int dvbsub_display_end_segment(AVCodecContext *avctx, const uint8_t *buf,
                                         int buf_size, AVSubtitle *sub)
 {
     DVBSubContext *ctx = (DVBSubContext*) avctx->priv_data;
+    DVBSubDisplayDefinition *display_def = ctx->display_definition;
 
     DVBSubRegion *region;
     DVBSubRegionDisplay *display;
@@ -1265,12 +1320,18 @@ static int dvbsub_display_end_segment(AVCodecContext *avctx, const uint8_t *buf,
     DVBSubCLUT *clut;
     uint32_t *clut_table;
     int i;
+    int offset_x=0, offset_y=0;
 
     sub->rects = NULL;
     sub->start_display_time = 0;
     sub->end_display_time = ctx->time_out * 1000;
     sub->format = 0;
 
+    if (display_def) {
+        offset_x = display_def->x;
+        offset_y = display_def->y;
+    }
+
     sub->num_rects = ctx->display_list_size;
 
     if (sub->num_rects > 0){
@@ -1288,8 +1349,8 @@ static int dvbsub_display_end_segment(AVCodecContext *avctx, const uint8_t *buf,
         if (!region)
             continue;
 
-        rect->x = display->x_pos;
-        rect->y = display->y_pos;
+        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 = 16;
@@ -1389,6 +1450,8 @@ static int dvbsub_decode(AVCodecContext *avctx,
             case DVBSUB_OBJECT_SEGMENT:
                 dvbsub_parse_object_segment(avctx, p, segment_length);
                 break;
+            case DVBSUB_DISPLAYDEFINITION_SEGMENT:
+                dvbsub_parse_display_definition_segment(avctx, p, segment_length);
             case DVBSUB_DISPLAY_SEGMENT:
                 *data_size = dvbsub_display_end_segment(avctx, p, segment_length, sub);
                 break;



More information about the ffmpeg-devel mailing list