ext/annodex/gstannodex.c: Do some extra sanity checks.

Original commit message from CVS:
Patch by: Alessandro Decina <alessandro at nnva dot org>
* ext/annodex/gstannodex.c: (gst_annodex_granule_to_time):
Do some extra sanity checks.
Fixes #350340.
* ext/annodex/gstcmmlenc.c: (gst_cmml_enc_change_state),
(gst_cmml_enc_parse_tag_head), (gst_cmml_enc_parse_tag_clip),
(gst_cmml_enc_push_clip), (gst_cmml_enc_push):
Check if clip->start_time is valid before adding the clip to the
track list.
Reset enc->preamble going from PAUSED to READY.
Don't use GST_FLOW_UNEXPECTED for wrong usage of the element, it is
only used for EOS.
Only post an error message if we were the one that created the fatal
GstFlowReturn value.
* ext/annodex/gstcmmlutils.c: (gst_cmml_clock_time_from_npt),
(gst_cmml_clock_time_to_granule), (gst_cmml_track_list_has_clip):
Parse the seconds field of the npt-sec time format using %llu rather than
%d and check that the value scaled by GST_SECOND doesn't overflow.
Use guint64(s) to represent the keyindex and keyoffset fields of a granulepos.
Lookup a clip's track with clip->track rather than clip->id which
makes no sense.
Identify a clip by its track and start time and not its xml id.
do some more input checking and make sure we don't do undefined shifts.
* tests/check/elements/cmmldec.c: (setup_cmmldec),
(teardown_cmmldec), (check_output_buffer_is_equal), (push_data),
(cmml_tag_message_pop), (check_headers), (push_clip_full),
(push_clip), (push_empty_clip), (check_output_clip),
(GST_START_TEST), (cmmldec_suite):
* tests/check/elements/cmmlenc.c: (setup_cmmlenc),
(teardown_cmmlenc), (check_output_buffer_is_equal), (push_data),
(check_headers), (push_clip), (check_clip_times), (check_clip),
(check_empty_clip), (GST_START_TEST), (cmmlenc_suite):
Added some more checks.

3 files changed, 37 insertions, 21 deletions

diff --git a/ext/annodex/gstannodex.c b/ext/annodex/gstannodex.c
index fa935f25..22fe798d 100644
--- a/ext/annodex/gstannodex.c
+++ b/ext/annodex/gstannodex.c
@@ -39,13 +39,15 @@ gst_annodex_granule_to_time (gint64 granulepos, gint64 granulerate_n,
   gint64 granulerate;
   GstClockTime res;
 
+  g_return_val_if_fail (granuleshift <= 64, GST_CLOCK_TIME_NONE);
+
   if (granulepos == -1)
     return GST_CLOCK_TIME_NONE;
 
   if (granulepos == 0 || granulerate_n == 0 || granulerate_d == 0)
     return 0;
 
-  if (granuleshift != 0) {
+  if (granuleshift != 0 && granuleshift != 64) {
     keyindex = granulepos >> granuleshift;
     keyoffset = granulepos - (keyindex << granuleshift);
     granulepos = keyindex + keyoffset;
diff --git a/ext/annodex/gstcmmlenc.c b/ext/annodex/gstcmmlenc.c
index d8392c63..9b2c03c6 100644
--- a/ext/annodex/gstcmmlenc.c
+++ b/ext/annodex/gstcmmlenc.c
@@ -259,6 +259,7 @@ gst_cmml_enc_change_state (GstElement * element, GstStateChange transition)
       gst_cmml_track_list_destroy (enc->tracks);
       enc->tracks = NULL;
       g_free (enc->preamble);
+      enc->preamble = NULL;
       gst_cmml_parser_free (enc->parser);
       break;
     }
@@ -468,7 +469,7 @@ gst_cmml_enc_parse_tag_head (GstCmmlEnc * enc, GstCmmlTagHead * head)
 flow_unexpected:
   GST_ELEMENT_ERROR (enc, STREAM, ENCODE,
       (NULL), ("got head tag before preamble"));
-  enc->flow_return = GST_FLOW_UNEXPECTED;
+  enc->flow_return = GST_FLOW_ERROR;
   return;
 push_error:
   gst_caps_unref (caps);
@@ -507,6 +508,14 @@ gst_cmml_enc_parse_tag_clip (GstCmmlEnc * enc, GstCmmlTagClip * clip)
       (gchar *) clip->track);
   if (prev_clip) {
     prev_clip_time = prev_clip->start_time;
+    if (prev_clip_time > clip->start_time) {
+      GST_ELEMENT_ERROR (enc, STREAM, ENCODE,
+          (NULL), ("previous clip start time > current clip (%s) start time",
+              clip->id));
+      enc->flow_return = GST_FLOW_ERROR;
+      return;
+    }
+
     /* we don't need the prev clip anymore */
     gst_cmml_track_list_del_clip (enc->tracks, prev_clip);
   }
@@ -526,12 +535,6 @@ gst_cmml_enc_push_clip (GstCmmlEnc * enc, GstCmmlTagClip * clip,
   gchar *clip_string;
   gint64 granulepos;
 
-  if (prev_clip_time != GST_CLOCK_TIME_NONE &&
-      prev_clip_time > clip->start_time) {
-    GST_WARNING_OBJECT (enc,
-        "previous clip start time > current clip (%s) start time", clip->id);
-  }
-
   /* encode the clip */
   clip_string =
       (gchar *) gst_cmml_parser_tag_clip_to_string (enc->parser, clip);
@@ -591,8 +594,7 @@ gst_cmml_enc_push (GstCmmlEnc * enc, GstBuffer * buffer)
 
   res = gst_pad_push (enc->srcpad, buffer);
   if (GST_FLOW_IS_FATAL (res))
-    GST_ELEMENT_ERROR (enc, STREAM, ENCODE,
-        (NULL), ("could not push buffer: %s", gst_flow_get_name (res)));
+    GST_WARNING_OBJECT (enc, "push returned: %s", gst_flow_get_name (res));
 
   return res;
 }
diff --git a/ext/annodex/gstcmmlutils.c b/ext/annodex/gstcmmlutils.c
index a9e561f8..48f7fb05 100644
--- a/ext/annodex/gstcmmlutils.c
+++ b/ext/annodex/gstcmmlutils.c
@@ -58,15 +58,17 @@ gst_cmml_clock_time_from_npt (const gchar * time)
 
     seconds_t = seconds * GST_SECOND;
   } else {
+    guint64 u64seconds;
+
     /* parse npt-sec */
     hours_t = 0;
     minutes = 0;
-    fields = sscanf (time, "%d.%d", &seconds, &mseconds);
+    fields = sscanf (time, "%llu.%d", &u64seconds, &mseconds);
     if (seconds < 0)
       goto bad_input;
 
-    seconds_t = gst_util_uint64_scale (seconds, GST_SECOND, 1);
-    if (seconds == G_MAXUINT64)
+    seconds_t = gst_util_uint64_scale_int (u64seconds, GST_SECOND, 1);
+    if (seconds_t == G_MAXUINT64)
       goto overflow;
   }
 
@@ -177,9 +179,13 @@ gst_cmml_clock_time_to_granule (GstClockTime prev_time,
     GstClockTime current_time, gint64 granulerate_n, gint64 granulerate_d,
     guint8 granuleshift)
 {
-  gint64 keyindex, keyoffset, granulepos;
+  guint64 keyindex, keyoffset, granulepos, maxoffset;
   gint64 granulerate;
 
+  g_return_val_if_fail (granulerate_d != 0, -1);
+  g_return_val_if_fail (granuleshift > 0, -1);
+  g_return_val_if_fail (granuleshift <= 64, -1);
+
   if (prev_time == GST_CLOCK_TIME_NONE)
     prev_time = 0;
 
@@ -191,14 +197,23 @@ gst_cmml_clock_time_to_granule (GstClockTime prev_time,
       granulerate_d, granulerate_n);
 
   prev_time = prev_time / granulerate;
-  if (prev_time > (((guint64) 1 << (64 - granuleshift)) - 1))
+
+  /* granuleshift == 64 should be a << 0 shift, which is defined */
+  maxoffset = ((guint64) 1 << (64 - granuleshift)) - 1;
+  if (prev_time > maxoffset)
     /* we need more than 64 - granuleshift bits to encode prev_time */
     goto overflow;
 
   keyindex = prev_time << granuleshift;
 
   keyoffset = (current_time / granulerate) - prev_time;
-  if (keyoffset > ((guint64) 1 << granuleshift) - 1)
+  /* make sure we don't shift to the limits of the types as this is undefined. */
+  if (granuleshift == 64)
+    maxoffset = G_MAXUINT64;
+  else
+    maxoffset = ((guint64) 1 << granuleshift) - 1;
+
+  if (keyoffset > maxoffset)
     /* we need more than granuleshift bits to encode prev_time - current_time */
     goto overflow;
 
@@ -301,16 +316,13 @@ gst_cmml_track_list_has_clip (GHashTable * tracks, GstCmmlTagClip * clip)
   GstCmmlTrack *track;
   GList *walk;
   GstCmmlTagClip *tmp;
-  gchar *clip_id = (gchar *) clip->id;
   gboolean res = FALSE;
 
-  g_return_val_if_fail (clip_id != NULL, FALSE);
-
-  track = g_hash_table_lookup (tracks, clip_id);
+  track = g_hash_table_lookup (tracks, (gchar *) clip->track);
   if (track) {
     for (walk = track->clips; walk; walk = g_list_next (walk)) {
       tmp = GST_CMML_TAG_CLIP (walk->data);
-      if (!strcmp ((gchar *) tmp->id, clip_id)) {
+      if (tmp->start_time == clip->start_time) {
         res = TRUE;
         break;
       }