codecs: Add minimal state validation
authorSeungha Yang <seungha@centricular.com>
Mon, 23 Mar 2020 05:40:52 +0000 (14:40 +0900)
committerGStreamer Merge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Thu, 9 Apr 2020 19:33:56 +0000 (19:33 +0000)
... to prevent requesting decoding before the preparation.
For instance, baseclass should not request decoding a picture if there
is no parsed valid headers, since subclass is most likely
not ready to decoding it.

gst-libs/gst/codecs/gsth264decoder.c
gst-libs/gst/codecs/gsth265decoder.c
gst-libs/gst/codecs/gstvp9decoder.c

index a235d74..a53187b 100644 (file)
@@ -340,9 +340,9 @@ gst_h264_decoder_parse_sps (GstH264Decoder * self, GstH264NalUnit * nalu)
   GstH264DecoderPrivate *priv = self->priv;
   GstH264SPS sps;
   GstH264ParserResult pres;
-  gboolean ret = TRUE;
+  gboolean ret;
 
-  pres = gst_h264_parser_parse_sps (priv->parser, nalu, &sps);
+  pres = gst_h264_parse_sps (nalu, &sps);
   if (pres != GST_H264_PARSER_OK) {
     GST_WARNING_OBJECT (self, "Failed to parse SPS, result %d", pres);
     return FALSE;
@@ -350,8 +350,14 @@ gst_h264_decoder_parse_sps (GstH264Decoder * self, GstH264NalUnit * nalu)
 
   GST_LOG_OBJECT (self, "SPS parsed");
 
-  if (!gst_h264_decoder_process_sps (self, &sps))
+  ret = gst_h264_decoder_process_sps (self, &sps);
+  if (!ret) {
+    GST_WARNING_OBJECT (self, "Failed to process SPS");
+  } else if (gst_h264_parser_update_sps (priv->parser,
+          &sps) != GST_H264_PARSER_OK) {
+    GST_WARNING_OBJECT (self, "Failed to update SPS");
     ret = FALSE;
+  }
 
   gst_h264_sps_clear (&sps);
 
@@ -424,7 +430,10 @@ gst_h264_decoder_parse_codec_data (GstH264Decoder * self, const guint8 * data,
       return FALSE;
     }
 
-    gst_h264_decoder_parse_sps (self, &nalu);
+    if (!gst_h264_decoder_parse_sps (self, &nalu)) {
+      GST_WARNING_OBJECT (self, "Failed to parse SPS");
+      return FALSE;
+    }
     off = nalu.offset + nalu.size;
   }
 
@@ -444,7 +453,10 @@ gst_h264_decoder_parse_codec_data (GstH264Decoder * self, const guint8 * data,
       return FALSE;
     }
 
-    gst_h264_decoder_parse_pps (self, &nalu);
+    if (!gst_h264_decoder_parse_pps (self, &nalu)) {
+      GST_WARNING_OBJECT (self, "Failed to parse PPS");
+      return FALSE;
+    }
     off = nalu.offset + nalu.size;
   }
 
@@ -830,7 +842,11 @@ gst_h264_decoder_set_format (GstVideoDecoder * decoder,
     GstMapInfo map;
 
     gst_buffer_map (priv->codec_data, &map, GST_MAP_READ);
-    gst_h264_decoder_parse_codec_data (self, map.data, map.size);
+    if (!gst_h264_decoder_parse_codec_data (self, map.data, map.size)) {
+      /* keep going without error.
+       * Probably inband SPS/PPS might be valid data */
+      GST_WARNING_OBJECT (self, "Failed to handle codec data");
+    }
     gst_buffer_unmap (priv->codec_data, &map);
   }
 
index 9203369..b06c8a1 100644 (file)
@@ -281,9 +281,9 @@ gst_h265_decoder_parse_sps (GstH265Decoder * self, GstH265NalUnit * nalu)
   GstH265DecoderPrivate *priv = self->priv;
   GstH265SPS sps;
   GstH265ParserResult pres;
-  gboolean ret = TRUE;
+  gboolean ret;
 
-  pres = gst_h265_parser_parse_sps (priv->parser, nalu, &sps, TRUE);
+  pres = gst_h265_parse_sps (priv->parser, nalu, &sps, TRUE);
   if (pres != GST_H265_PARSER_OK) {
     GST_WARNING_OBJECT (self, "Failed to parse SPS, result %d", pres);
     return FALSE;
@@ -291,8 +291,14 @@ gst_h265_decoder_parse_sps (GstH265Decoder * self, GstH265NalUnit * nalu)
 
   GST_LOG_OBJECT (self, "SPS parsed");
 
-  if (!gst_h265_decoder_process_sps (self, &sps))
+  ret = gst_h265_decoder_process_sps (self, &sps);
+  if (!ret) {
+    GST_WARNING_OBJECT (self, "Failed to process SPS");
+  } else if (gst_h265_parser_update_sps (priv->parser,
+          &sps) != GST_H265_PARSER_OK) {
+    GST_WARNING_OBJECT (self, "Failed to update SPS");
     ret = FALSE;
+  }
 
   return ret;
 }
@@ -564,13 +570,22 @@ gst_h265_decoder_parse_codec_data (GstH265Decoder * self, const guint8 * data,
 
       switch (nalu.type) {
         case GST_H265_NAL_VPS:
-          gst_h265_decoder_parse_vps (self, &nalu);
+          if (!gst_h265_decoder_parse_vps (self, &nalu)) {
+            GST_WARNING_OBJECT (self, "Failed to parse VPS");
+            return FALSE;
+          }
           break;
         case GST_H265_NAL_SPS:
-          gst_h265_decoder_parse_sps (self, &nalu);
+          if (!gst_h265_decoder_parse_sps (self, &nalu)) {
+            GST_WARNING_OBJECT (self, "Failed to parse SPS");
+            return FALSE;
+          }
           break;
         case GST_H265_NAL_PPS:
-          gst_h265_decoder_parse_pps (self, &nalu);
+          if (!gst_h265_decoder_parse_pps (self, &nalu)) {
+            GST_WARNING_OBJECT (self, "Failed to parse PPS");
+            return FALSE;
+          }
           break;
         default:
           break;
@@ -657,7 +672,11 @@ gst_h265_decoder_set_format (GstVideoDecoder * decoder,
     GstMapInfo map;
 
     gst_buffer_map (priv->codec_data, &map, GST_MAP_READ);
-    gst_h265_decoder_parse_codec_data (self, map.data, map.size);
+    if (!gst_h265_decoder_parse_codec_data (self, map.data, map.size)) {
+      /* keep going without error.
+       * Probably inband SPS/PPS might be valid data */
+      GST_WARNING_OBJECT (self, "Failed to handle codec data");
+    }
     gst_buffer_unmap (priv->codec_data, &map);
   }
 
index 5d80af5..e4d68f9 100644 (file)
@@ -186,9 +186,10 @@ gst_vp9_decoder_check_codec_change (GstVp9Decoder * self,
     GstVp9DecoderClass *klass = GST_VP9_DECODER_GET_CLASS (self);
 
     priv->had_sequence = TRUE;
-
     if (klass->new_sequence)
-      ret = klass->new_sequence (self, frame_hdr);
+      priv->had_sequence = klass->new_sequence (self, frame_hdr);
+
+    ret = priv->had_sequence;
   }
 
   return ret;
@@ -346,6 +347,17 @@ gst_vp9_decoder_handle_frame (GstVideoDecoder * decoder,
     return gst_video_decoder_drop_frame (decoder, frame);;
   }
 
+  if (frame_hdr[0].frame_type == GST_VP9_KEY_FRAME &&
+      !gst_vp9_decoder_check_codec_change (self, &frame_hdr[0])) {
+    GST_ERROR_OBJECT (self, "codec change error");
+    goto unmap_and_error;
+  }
+
+  if (!priv->had_sequence) {
+    GST_WARNING_OBJECT (self, "No handled frame header, drop frame");
+    goto unmap_and_error;
+  }
+
   priv->wait_keyframe = FALSE;
 
   offset = 0;
@@ -386,12 +398,6 @@ gst_vp9_decoder_handle_frame (GstVideoDecoder * decoder,
       gst_vp9_picture_unref (picture);
       picture = NULL;
     } else {
-      if (cur_hdr->frame_type == GST_VP9_KEY_FRAME &&
-          !gst_vp9_decoder_check_codec_change (self, cur_hdr)) {
-        GST_ERROR_OBJECT (self, "codec change error");
-        goto unmap_and_error;
-      }
-
       picture = gst_vp9_picture_new ();
       picture->frame_hdr = *cur_hdr;
       picture->pts = GST_BUFFER_PTS (in_buf);