[FFmpeg-devel] [PATCH] 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 incorrect condition prevented proper fallback to using the default
resolution for the decoding context.

This also adds variables and moves the fallback check to the outside
for better clarity.

Signed-off-by: softworkz <softworkz at hotmail.com>
---
    avcodec/dvbsubdec: Fix conditions for fallback to default resolution
    
    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 incorrect condition prevented proper fallback to using the default
    resolution for the decoding context.
    
    This also adds variables and moves the fallback check to the outside for
    better clarity.

Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-57%2Fsoftworkz%2Fsubmit_dvbsubdec-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-57/softworkz/submit_dvbsubdec-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/57

 libavcodec/dvbsubdec.c | 50 ++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index 4553c45b3d..cbb16e501a 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -35,7 +35,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)
 
@@ -1451,8 +1451,11 @@ 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_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");
 
@@ -1497,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",
@@ -1537,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) {
+
+        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;

base-commit: 79c47dfd25f101b6842bbec8c6ffef8d5077c3ae
-- 
ffmpeg-codebot