[FFmpeg-devel] [PATCH v6 25/25] avcodec/dvbsubdec: Fix conditions for fallback to default resolution

[softworkz]

From: softworkz <softworkz at hotmail.com>

The previous code expected a segment of type CLUT definition to exist
in order to accept a set of segments to be complete.
This was an incorrect assumption as the presence of a CLUT segment
is not mandatory.
(version 1.6.1 of the spec is probably a bit more clear about this
than earlier versions: https://www.etsi.org/deliver/etsi_en/
300700_300799/300743/01.06.01_20/en_300743v010601a.pdf)

The flawed condition prevented proper fallback to using the default
resolution for the decoding context.

Signed-off-by: softworkz <softworkz at hotmail.com>
---
 libavcodec/dvbsubdec.c | 51 +++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index 8d44529b4a..06f39161a8 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -34,7 +34,7 @@
 #define DVBSUB_CLUT_SEGMENT     0x12
 #define DVBSUB_OBJECT_SEGMENT   0x13
 #define DVBSUB_DISPLAYDEFINITION_SEGMENT 0x14
-#define DVBSUB_DISPLAY_SEGMENT  0x80
+#define DVBSUB_END_DISPLAY_SEGMENT  0x80
 
 #define cm (ff_crop_tab + MAX_NEG_CROP)
 
@@ -1450,8 +1450,12 @@ static int dvbsub_decode(AVCodecContext *avctx, AVSubtitle *sub,
     int segment_length;
     int i;
     int ret = 0;
-    int got_segment = 0;
-    int got_dds = 0;
+    //int got_segment = 0;
+    int got_page = 0;
+    int got_region = 0;
+    int got_object = 0;
+    int got_end_display = 0;
+    int got_displaydef = 0;
 
     ff_dlog(avctx, "DVB sub packet:\n");
 
@@ -1496,34 +1500,28 @@ static int dvbsub_decode(AVCodecContext *avctx, AVSubtitle *sub,
             switch (segment_type) {
             case DVBSUB_PAGE_SEGMENT:
                 ret = dvbsub_parse_page_segment(avctx, p, segment_length, sub, got_sub_ptr);
-                got_segment |= 1;
+                got_page = 1;
                 break;
             case DVBSUB_REGION_SEGMENT:
                 ret = dvbsub_parse_region_segment(avctx, p, segment_length);
-                got_segment |= 2;
+                got_region = 1;
                 break;
             case DVBSUB_CLUT_SEGMENT:
                 ret = dvbsub_parse_clut_segment(avctx, p, segment_length);
                 if (ret < 0) goto end;
-                got_segment |= 4;
                 break;
             case DVBSUB_OBJECT_SEGMENT:
                 ret = dvbsub_parse_object_segment(avctx, p, segment_length);
-                got_segment |= 8;
+                got_object = 1;
                 break;
             case DVBSUB_DISPLAYDEFINITION_SEGMENT:
                 ret = dvbsub_parse_display_definition_segment(avctx, p,
                                                               segment_length);
-                got_dds = 1;
+                got_displaydef = 1;
                 break;
-            case DVBSUB_DISPLAY_SEGMENT:
+            case DVBSUB_END_DISPLAY_SEGMENT:
                 ret = dvbsub_display_end_segment(avctx, p, segment_length, sub, got_sub_ptr);
-                if (got_segment == 15 && !got_dds && !avctx->width && !avctx->height) {
-                    // Default from ETSI EN 300 743 V1.3.1 (7.2.1)
-                    avctx->width  = 720;
-                    avctx->height = 576;
-                }
-                got_segment |= 16;
+                got_end_display = 1;
                 break;
             default:
                 ff_dlog(avctx, "Subtitling segment type 0x%x, page id %d, length %d\n",
@@ -1536,13 +1534,24 @@ static int dvbsub_decode(AVCodecContext *avctx, AVSubtitle *sub,
 
         p += segment_length;
     }
-    // Some streams do not send a display segment but if we have all the other
-    // segments then we need no further data.
-    if (got_segment == 15) {
-        av_log(avctx, AV_LOG_DEBUG, "Missing display_end_segment, emulating\n");
-        dvbsub_display_end_segment(avctx, p, 0, sub, got_sub_ptr);
-    }
 
+    // Even though not mandated by the spec, we're imposing a minimum requirement
+    // for a useful packet to have at least one page, region and object segment.
+    if (got_page && got_region && got_object && got_end_display) {
+
+        if (!got_displaydef && !avctx->width && !avctx->height) {
+            // Default from ETSI EN 300 743 V1.3.1 (7.2.1)
+            avctx->width  = 720;
+            avctx->height = 576;
+        }
+
+        // Some streams do not send an end-of-display segment but if we have all the other
+        // segments then we need no further data.
+        if (!got_end_display) {
+            av_log(avctx, AV_LOG_DEBUG, "Missing display_end_segment, emulating\n");
+            dvbsub_display_end_segment(avctx, p, 0, sub, got_sub_ptr);
+        }
+    }
 end:
     if (ret < 0) {
         return ret;
-- 
ffmpeg-codebot