isomp4: Handle mp4s subpicture streams better.

Clean up the handling of mp4s streams. Use the generic esds
descriptor function to extract the palette, instead of hard coding
a wrong magic offset.

Add some more size safety checks when parsing ES descriptors, and
replace magic numbers with the descriptive constants that are already
defined.

Enhance dump output for stsd atoms.

Streams from both bug 712643 and historic bug 568278 now both work
correctly.

Fixes: #712643

diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c
index cc5df22482e5ad2ce342d1cfd6dbfad24881b3a5..a959a56c616edaf91327c4a0da7fb053fbdbf3ca 100644 (file)
--- a/gst/isomp4/qtdemux.c
+++ b/gst/isomp4/qtdemux.c
@@ -58,6 +58,7 @@
 #include "qtdemux_types.h"
 #include "qtdemux_dump.h"
 #include "fourcc.h"
+#include "descriptors.h"
 #include "qtdemux_lang.h"
 #include "qtdemux.h"
 #include "qtpalette.h"
@@ -5372,6 +5373,7 @@ qtdemux_parse_node (GstQTDemux * qtdemux, GNode * node, const guint8 * buffer,
         }
         GST_DEBUG_OBJECT (qtdemux,
             "parsing stsd (sample table, sample description) atom");
+        /* Skip over 8 byte atom hdr + 1 byte version, 3 bytes flags, 4 byte num_entries */
         qtdemux_parse_container (qtdemux, node, buffer + 16, end);
         break;
       }
@@ -5519,6 +5521,13 @@ qtdemux_parse_node (GstQTDemux * qtdemux, GNode * node, const guint8 * buffer,
         qtdemux_parse_container (qtdemux, node, buffer + 12, end);
         break;
       }
+      case FOURCC_mp4s:
+      {
+        GST_MEMDUMP_OBJECT (qtdemux, "mp4s", buffer, end - buffer);
+        /* Skip 8 byte header, plus 8 byte version + flags + entry_count */
+        qtdemux_parse_container (qtdemux, node, buffer + 16, end);
+        break;
+      }
       case FOURCC_XiTh:
       {
         guint32 version;
@@ -8360,46 +8369,20 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
     switch (fourcc) {
       case FOURCC_mp4s:
       {
-        guint len;
-        const guint8 *data;
-
-        /* look for palette */
-        /* target mp4s atom */
-        len = QT_UINT32 (stsd_data + offset);
-        data = stsd_data + offset;
-        /* verify sufficient length,
-         * and esds present with decConfigDescr of expected size and position */
-        if ((len >= 106 + 8)
-            && (QT_FOURCC (data + 8 + 8 + 4) == FOURCC_esds)
-            && (QT_UINT16 (data + 8 + 40) == 0x0540)) {
-          GstStructure *s;
-          guint32 clut[16];
-          gint i;
-
-          /* move to decConfigDescr data */
-          data = data + 8 + 42;
-          for (i = 0; i < 16; i++) {
-            clut[i] = QT_UINT32 (data);
-            data += 4;
-          }
-
-          s = gst_structure_new ("application/x-gst-dvd", "event",
-              G_TYPE_STRING, "dvd-spu-clut-change",
-              "clut00", G_TYPE_INT, clut[0], "clut01", G_TYPE_INT, clut[1],
-              "clut02", G_TYPE_INT, clut[2], "clut03", G_TYPE_INT, clut[3],
-              "clut04", G_TYPE_INT, clut[4], "clut05", G_TYPE_INT, clut[5],
-              "clut06", G_TYPE_INT, clut[6], "clut07", G_TYPE_INT, clut[7],
-              "clut08", G_TYPE_INT, clut[8], "clut09", G_TYPE_INT, clut[9],
-              "clut10", G_TYPE_INT, clut[10], "clut11", G_TYPE_INT, clut[11],
-              "clut12", G_TYPE_INT, clut[12], "clut13", G_TYPE_INT, clut[13],
-              "clut14", G_TYPE_INT, clut[14], "clut15", G_TYPE_INT, clut[15],
-              NULL);
-
-          /* store event and trigger custom processing */
-          stream->pending_event =
-              gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM, s);
-          stream->need_process = TRUE;
+        GNode *mp4s = NULL;
+        GNode *esds = NULL;
+
+        /* look for palette in a stsd->mp4s->esds sub-atom */
+        mp4s = qtdemux_tree_get_child_by_type (stsd, FOURCC_mp4s);
+        if (mp4s)
+          esds = qtdemux_tree_get_child_by_type (mp4s, FOURCC_esds);
+        if (esds == NULL) {
+          /* Invalid STSD */
+          GST_LOG_OBJECT (qtdemux, "Skipping invalid stsd: no esds child");
+          break;
         }
+
+        gst_qtdemux_handle_esds (qtdemux, stream, esds, list);
         break;
       }
       default:
@@ -8410,7 +8393,6 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
     GST_INFO_OBJECT (qtdemux,
         "type %" GST_FOURCC_FORMAT " caps %" GST_PTR_FORMAT,
         GST_FOURCC_ARGS (fourcc), stream->caps);
-
   } else {
     /* everything in 1 sample */
     stream->sampled = TRUE;
@@ -10063,22 +10045,24 @@ qtdemux_parse_tree (GstQTDemux * qtdemux)
 }
 
 /* taken from ffmpeg */
-static unsigned int
-get_size (guint8 * ptr, guint8 ** end)
+static int
+read_descr_size (guint8 * ptr, guint8 * end, guint8 ** end_out)
 {
   int count = 4;
   int len = 0;
 
   while (count--) {
-    int c = *ptr;
+    int c;
 
-    ptr++;
+    if (ptr >= end)
+      return -1;
+
+    c = *ptr++;
     len = (len << 7) | (c & 0x7f);
     if (!(c & 0x80))
       break;
   }
-  if (end)
-    *end = ptr;
+  *end_out = ptr;
   return len;
 }
 
@@ -10101,20 +10085,24 @@ gst_qtdemux_handle_esds (GstQTDemux * qtdemux, QtDemuxStream * stream,
   ptr += 8;
   GST_DEBUG_OBJECT (qtdemux, "version/flags = %08x", QT_UINT32 (ptr));
   ptr += 4;
-  while (ptr < end) {
+  while (ptr + 1 < end) {
     tag = QT_UINT8 (ptr);
     GST_DEBUG_OBJECT (qtdemux, "tag = %02x", tag);
     ptr++;
-    len = get_size (ptr, &ptr);
+    len = read_descr_size (ptr, end, &ptr);
     GST_DEBUG_OBJECT (qtdemux, "len = %d", len);
 
+    /* Check the stated amount of data is available for reading */
+    if (len < 0 || ptr + len > end)
+      break;
+
     switch (tag) {
-      case 0x03:
+      case ES_DESCRIPTOR_TAG:
         GST_DEBUG_OBJECT (qtdemux, "ID %04x", QT_UINT16 (ptr));
         GST_DEBUG_OBJECT (qtdemux, "priority %04x", QT_UINT8 (ptr + 2));
         ptr += 3;
         break;
-      case 0x04:{
+      case DECODER_CONFIG_DESC_TAG:{
         guint max_bitrate, avg_bitrate;
 
         object_type_id = QT_UINT8 (ptr);
@@ -10136,18 +10124,56 @@ gst_qtdemux_handle_esds (GstQTDemux * qtdemux, QtDemuxStream * stream,
         ptr += 13;
         break;
       }
-      case 0x05:
+      case DECODER_SPECIFIC_INFO_TAG:
         GST_MEMDUMP_OBJECT (qtdemux, "data", ptr, len);
-        data_ptr = ptr;
-        data_len = len;
+        if (object_type_id == 0xe0 && len == 0x40) {
+          guint8 *data;
+          GstStructure *s;
+          guint32 clut[16];
+          gint i;
+
+          GST_DEBUG_OBJECT (qtdemux,
+              "Have VOBSUB palette. Creating palette event");
+          /* move to decConfigDescr data and read palette */
+          data = ptr;
+          for (i = 0; i < 16; i++) {
+            clut[i] = QT_UINT32 (data);
+            data += 4;
+          }
+
+          s = gst_structure_new ("application/x-gst-dvd", "event",
+              G_TYPE_STRING, "dvd-spu-clut-change",
+              "clut00", G_TYPE_INT, clut[0], "clut01", G_TYPE_INT, clut[1],
+              "clut02", G_TYPE_INT, clut[2], "clut03", G_TYPE_INT, clut[3],
+              "clut04", G_TYPE_INT, clut[4], "clut05", G_TYPE_INT, clut[5],
+              "clut06", G_TYPE_INT, clut[6], "clut07", G_TYPE_INT, clut[7],
+              "clut08", G_TYPE_INT, clut[8], "clut09", G_TYPE_INT, clut[9],
+              "clut10", G_TYPE_INT, clut[10], "clut11", G_TYPE_INT, clut[11],
+              "clut12", G_TYPE_INT, clut[12], "clut13", G_TYPE_INT, clut[13],
+              "clut14", G_TYPE_INT, clut[14], "clut15", G_TYPE_INT, clut[15],
+              NULL);
+
+          /* store event and trigger custom processing */
+          stream->pending_event =
+              gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM, s);
+          stream->need_process = TRUE;
+        } else {
+          /* Generic codec_data handler puts it on the caps */
+          data_ptr = ptr;
+          data_len = len;
+        }
+
         ptr += len;
         break;
-      case 0x06:
+      case SL_CONFIG_DESC_TAG:
         GST_DEBUG_OBJECT (qtdemux, "data %02x", QT_UINT8 (ptr));
         ptr += 1;
         break;
       default:
-        GST_ERROR_OBJECT (qtdemux, "parse error");
+        GST_DEBUG_OBJECT (qtdemux, "Unknown/unhandled descriptor tag %02x",
+            tag);
+        GST_MEMDUMP_OBJECT (qtdemux, "descriptor data", ptr, len);
+        ptr += len;
         break;
     }
   }

diff --git a/gst/isomp4/qtdemux_dump.c b/gst/isomp4/qtdemux_dump.c
index 50e44de20eb0cdf80cb66e432f318610e748dd10..c76b0f21fe6f1d782132d2272484c7d697c5d07a 100644 (file)
--- a/gst/isomp4/qtdemux_dump.c
+++ b/gst/isomp4/qtdemux_dump.c
@@ -254,6 +254,52 @@ qtdemux_dump_dref (GstQTDemux * qtdemux, GstByteReader * data, int depth)
   return TRUE;
 }
 
+static gboolean
+qtdemux_dump_stsd_avc1 (GstQTDemux * qtdemux, GstByteReader * data, guint size,
+    int depth)
+{
+  guint32 fourcc;
+
+  /* Size of avc1 = 78 bytes */
+  if (size < (6 + 2 + 4 + 4 + 4 + 4 + 2 + 2 + 4 + 4 + 4 + 2 + 1 + 31 + 2 + 2))
+    return FALSE;
+
+  gst_byte_reader_skip_unchecked (data, 6);
+  GST_LOG_OBJECT (qtdemux, "%*s    data reference:%d", depth, "",
+      GET_UINT16 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    version/rev.:  %08x", depth, "",
+      GET_UINT32 (data));
+  fourcc = GET_FOURCC (data);
+  GST_LOG_OBJECT (qtdemux, "%*s    vendor:        %" GST_FOURCC_FORMAT, depth,
+      "", GST_FOURCC_ARGS (fourcc));
+  GST_LOG_OBJECT (qtdemux, "%*s    temporal qual: %u", depth, "",
+      GET_UINT32 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    spatial qual:  %u", depth, "",
+      GET_UINT32 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    width:         %u", depth, "",
+      GET_UINT16 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    height:        %u", depth, "",
+      GET_UINT16 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    horiz. resol:  %g", depth, "",
+      GET_FP32 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    vert. resol.:  %g", depth, "",
+      GET_FP32 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    data size:     %u", depth, "",
+      GET_UINT32 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    frame count:   %u", depth, "",
+      GET_UINT16 (data));
+  /* something is not right with this, it's supposed to be a string but it's
+   * not apparently, so just skip this for now */
+  gst_byte_reader_skip_unchecked (data, 1 + 31);
+  GST_LOG_OBJECT (qtdemux, "%*s    compressor:    (skipped)", depth, "");
+  GST_LOG_OBJECT (qtdemux, "%*s    depth:         %u", depth, "",
+      GET_UINT16 (data));
+  GST_LOG_OBJECT (qtdemux, "%*s    color table ID:%u", depth, "",
+      GET_UINT16 (data));
+
+  return TRUE;
+}
+
 gboolean
 qtdemux_dump_stsd (GstQTDemux * qtdemux, GstByteReader * data, int depth)
 {
@@ -268,40 +314,46 @@ qtdemux_dump_stsd (GstQTDemux * qtdemux, GstByteReader * data, int depth)
 
   for (i = 0; i < num_entries; i++) {
     GstByteReader sub;
-    guint32 size = 0, fourcc;
+    guint32 size, remain;
+    guint32 fourcc;
 
     if (!gst_byte_reader_get_uint32_be (data, &size) ||
         !qt_atom_parser_get_fourcc (data, &fourcc))
       return FALSE;
 
-    GST_LOG ("%*s    size:          %u", depth, "", size);
-    GST_LOG ("%*s    type:          %" GST_FOURCC_FORMAT, depth, "",
-        GST_FOURCC_ARGS (fourcc));
+    GST_LOG_OBJECT (qtdemux, "%*s    size:          %u", depth, "", size);
+    GST_LOG_OBJECT (qtdemux, "%*s    type:          %" GST_FOURCC_FORMAT, depth,
+        "", GST_FOURCC_ARGS (fourcc));
 
-    if (size < (6 + 2 + 4 + 4 + 4 + 4 + 2 + 2 + 4 + 4 + 4 + 2 + 1 + 31 + 2 + 2))
+    remain = gst_byte_reader_get_remaining (data);
+    /* Size includes the 8 bytes we just read: len & fourcc, then 8 bytes
+     * version, flags, entries_count */
+    if (size > remain + 8) {
+      GST_LOG_OBJECT (qtdemux,
+          "Not enough data left for this atom (have %u need %u)", remain, size);
       return FALSE;
+    }
+
+    qt_atom_parser_peek_sub (data, 0, size, &sub);
+    switch (fourcc) {
+      case FOURCC_avc1:
+        if (!qtdemux_dump_stsd_avc1 (qtdemux, &sub, size, depth + 1))
+          return FALSE;
+        break;
+      case FOURCC_mp4s:
+        if (!gst_byte_reader_get_uint32_be (&sub, &ver_flags) ||
+            !gst_byte_reader_get_uint32_be (&sub, &num_entries))
+          return FALSE;
+        if (!qtdemux_dump_unknown (qtdemux, &sub, depth + 1))
+          return FALSE;
+        break;
+      default:
+        /* Unknown stsd data, dump the bytes */
+        if (!qtdemux_dump_unknown (qtdemux, &sub, depth + 1))
+          return FALSE;
+        break;
+    }
 
-    qt_atom_parser_peek_sub (data, 0, 78, &sub);
-    gst_byte_reader_skip_unchecked (&sub, 6);
-    GST_LOG ("%*s    data reference:%d", depth, "", GET_UINT16 (&sub));
-    GST_LOG ("%*s    version/rev.:  %08x", depth, "", GET_UINT32 (&sub));
-    fourcc = GET_FOURCC (&sub);
-    GST_LOG ("%*s    vendor:        %" GST_FOURCC_FORMAT, depth, "",
-        GST_FOURCC_ARGS (fourcc));
-    GST_LOG ("%*s    temporal qual: %u", depth, "", GET_UINT32 (&sub));
-    GST_LOG ("%*s    spatial qual:  %u", depth, "", GET_UINT32 (&sub));
-    GST_LOG ("%*s    width:         %u", depth, "", GET_UINT16 (&sub));
-    GST_LOG ("%*s    height:        %u", depth, "", GET_UINT16 (&sub));
-    GST_LOG ("%*s    horiz. resol:  %g", depth, "", GET_FP32 (&sub));
-    GST_LOG ("%*s    vert. resol.:  %g", depth, "", GET_FP32 (&sub));
-    GST_LOG ("%*s    data size:     %u", depth, "", GET_UINT32 (&sub));
-    GST_LOG ("%*s    frame count:   %u", depth, "", GET_UINT16 (&sub));
-    /* something is not right with this, it's supposed to be a string but it's
-     * not apparently, so just skip this for now */
-    gst_byte_reader_skip_unchecked (&sub, 1 + 31);
-    GST_LOG ("%*s    compressor:    (skipped)", depth, "");
-    GST_LOG ("%*s    depth:         %u", depth, "", GET_UINT16 (&sub));
-    GST_LOG ("%*s    color table ID:%u", depth, "", GET_UINT16 (&sub));
     if (!gst_byte_reader_skip (data, size - (4 + 4)))
       return FALSE;
   }

diff --git a/gst/isomp4/qtdemux_types.c b/gst/isomp4/qtdemux_types.c
index cad14a9b7ec5de6e716654423f67128992af4046..8988af02504781cf3934762b9add49fa45da60c3 100644 (file)
--- a/gst/isomp4/qtdemux_types.c
+++ b/gst/isomp4/qtdemux_types.c
@@ -174,6 +174,7 @@ static const QtNodeType qt_node_types[] = {
   {FOURCC_avcC, "AV codec configuration container", 0},
   {FOURCC_avc1, "AV codec configuration v1", 0},
   {FOURCC_avc3, "AV codec configuration v3", 0},
+  {FOURCC_mp4s, "VOBSUB codec configuration", 0},
   {FOURCC_hvc1, "HEVC codec configuration", 0},
   {FOURCC_hev1, "HEVC codec configuration", 0},
   {FOURCC_hvcC, "HEVC codec configuration container", 0},