splitmuxsink: Don't send too many force key unit event

splitmuxsink should requst keyframe depending on configured
threshold and previously requested time in order to avoid too many
keyframe request.

diff --git a/gst/multifile/gstsplitmuxsink.c b/gst/multifile/gstsplitmuxsink.c
index 6351840..6d55a83 100644 (file)
--- a/gst/multifile/gstsplitmuxsink.c
+++ b/gst/multifile/gstsplitmuxsink.c
@@ -567,6 +567,7 @@ gst_splitmux_sink_init (GstSplitMuxSink * splitmux)
   splitmux->do_split_next_gop = FALSE;
   splitmux->times_to_split = gst_queue_array_new_for_struct (8, 8);
   splitmux->last_fku_time = GST_CLOCK_TIME_NONE;
+  splitmux->next_fku_time = GST_CLOCK_TIME_NONE;
 }
 
 static void
@@ -1342,6 +1343,7 @@ request_next_keyframe (GstSplitMuxSink * splitmux, GstBuffer * buffer,
   GstClockTime target_time;
   gboolean timecode_based = FALSE;
   GstClockTime next_max_tc_time = GST_CLOCK_TIME_NONE;
+  GstClockTime next_fku_time = GST_CLOCK_TIME_NONE;
 
   if (splitmux->tc_interval) {
     GstVideoTimeCodeMeta *tc_meta;
@@ -1361,6 +1363,14 @@ request_next_keyframe (GstSplitMuxSink * splitmux, GstBuffer * buffer,
     }
   }
 
+  /* even if we don't send keyframe request, this should be done here in order
+   * to calculate the threshold timecode */
+  if (timecode_based && !GST_CLOCK_TIME_IS_VALID (splitmux->threshold_timecode)) {
+    splitmux->threshold_timecode = next_max_tc_time - running_time;
+    GST_DEBUG_OBJECT (splitmux, "Calculated threshold timecode duration %"
+        GST_TIME_FORMAT, GST_TIME_ARGS (splitmux->threshold_timecode));
+  }
+
   if (splitmux->send_keyframe_requests == FALSE
       || (splitmux->threshold_time == 0 && !timecode_based)
       || splitmux->threshold_bytes != 0)
@@ -1373,25 +1383,28 @@ request_next_keyframe (GstSplitMuxSink * splitmux, GstBuffer * buffer,
     target_time = running_time + splitmux->threshold_time;
   }
 
-  if (GST_CLOCK_TIME_IS_VALID (splitmux->last_fku_time) &&
-      splitmux->last_fku_time > target_time) {
+  if (GST_CLOCK_TIME_IS_VALID (splitmux->next_fku_time) &&
+      target_time < splitmux->next_fku_time) {
     GST_DEBUG_OBJECT (splitmux, "Target time %" GST_TIME_FORMAT
-        " is smaller than last requested keyframe time %" GST_TIME_FORMAT,
-        GST_TIME_ARGS (target_time), GST_TIME_ARGS (splitmux->last_fku_time));
+        " is smaller than expected next keyframe time %" GST_TIME_FORMAT,
+        GST_TIME_ARGS (target_time), GST_TIME_ARGS (splitmux->next_fku_time));
 
     return TRUE;
   }
 
-  splitmux->last_fku_time = target_time;
-  if (timecode_based && !GST_CLOCK_TIME_IS_VALID (splitmux->threshold_timecode)) {
-    splitmux->threshold_timecode = next_max_tc_time - running_time;
-    GST_DEBUG_OBJECT (splitmux, "Calculated threshold timecode duration %"
-        GST_TIME_FORMAT, GST_TIME_ARGS (splitmux->threshold_timecode));
+  if (timecode_based) {
+    next_fku_time = target_time + splitmux->threshold_timecode;
+  } else {
+    next_fku_time = target_time + splitmux->threshold_time;
   }
 
+  splitmux->last_fku_time = target_time;
+  splitmux->next_fku_time = next_fku_time;
+
   ev = gst_video_event_new_upstream_force_key_unit (target_time, TRUE, 0);
-  GST_INFO_OBJECT (splitmux, "Requesting next keyframe at %" GST_TIME_FORMAT,
-      GST_TIME_ARGS (target_time));
+  GST_INFO_OBJECT (splitmux, "Requesting keyframe at %" GST_TIME_FORMAT
+      ", the next expected keyframe is %" GST_TIME_FORMAT,
+      GST_TIME_ARGS (target_time), GST_TIME_ARGS (next_fku_time));
   return gst_pad_push_event (splitmux->reference_ctx->sinkpad, ev);
 }
 
@@ -3275,6 +3288,7 @@ gst_splitmux_sink_reset (GstSplitMuxSink * splitmux)
   g_atomic_int_set (&(splitmux->do_split_next_gop), FALSE);
 
   splitmux->last_fku_time = GST_CLOCK_TIME_NONE;
+  splitmux->next_fku_time = GST_CLOCK_TIME_NONE;
   gst_queue_array_clear (splitmux->times_to_split);
 
   g_list_foreach (splitmux->contexts, (GFunc) mq_stream_ctx_reset, NULL);

diff --git a/gst/multifile/gstsplitmuxsink.h b/gst/multifile/gstsplitmuxsink.h
index fe4118e..62665d9 100644 (file)
--- a/gst/multifile/gstsplitmuxsink.h
+++ b/gst/multifile/gstsplitmuxsink.h
@@ -122,7 +122,10 @@ struct _GstSplitMuxSink
   GstClockTime threshold_timecode;
   GstClockTime next_max_tc_time;
   GstClockTime alignment_threshold;
+  /* previously sent running time of force keyframe unit event */
   GstClockTime last_fku_time;
+  /* expected running time of next force keyframe unit event */
+  GstClockTime next_fku_time;
 
   gboolean reset_muxer;
 

diff --git a/tests/check/elements/splitmux.c b/tests/check/elements/splitmux.c
index 04c352c..be8c5c9 100644 (file)
--- a/tests/check/elements/splitmux.c
+++ b/tests/check/elements/splitmux.c
@@ -27,6 +27,7 @@
 
 #include <gst/check/gstcheck.h>
 #include <gst/app/app.h>
+#include <gst/video/video.h>
 #include <stdlib.h>
 
 gchar *tmpdir = NULL;
@@ -930,6 +931,24 @@ GST_START_TEST (test_splitmuxsink_muxer_pad_map)
 
 GST_END_TEST;
 
+static GstPadProbeReturn
+count_upstrea_fku (GstPad * pad, GstPadProbeInfo * info,
+    guint * upstream_fku_count)
+{
+  GstEvent *event = GST_PAD_PROBE_INFO_EVENT (info);
+
+  switch (GST_EVENT_TYPE (event)) {
+    case GST_EVENT_CUSTOM_UPSTREAM:
+      if (gst_video_event_is_force_key_unit (event))
+        *upstream_fku_count += 1;
+      break;
+    default:
+      break;
+  }
+
+  return GST_PAD_PROBE_OK;
+}
+
 static void
 splitmuxsink_split_by_keyframe (gboolean send_keyframe_request,
     guint max_size_time_sec, guint encoder_key_interval_sec)
@@ -937,17 +956,21 @@ splitmuxsink_split_by_keyframe (gboolean send_keyframe_request,
   GstMessage *msg;
   GstElement *pipeline;
   GstElement *sink;
+  GstElement *enc;
+  GstPad *srcpad;
   gchar *pipeline_str;
   gchar *dest_pattern;
   guint count;
   guint expected_count;
   gchar *in_pattern;
+  guint upstream_fku_count = 0;
+  guint expected_fku_count;
 
   pipeline_str = g_strdup_printf ("splitmuxsink name=splitsink "
       "max-size-time=%" G_GUINT64_FORMAT
       " send-keyframe-requests=%s muxer=qtmux "
       "videotestsrc num-buffers=30 ! video/x-raw,width=80,height=64,framerate=5/1 "
-      "! videoconvert ! queue ! vp8enc keyframe-max-dist=%d ! splitsink.video ",
+      "! videoconvert ! queue ! vp8enc name=enc keyframe-max-dist=%d ! splitsink.video ",
       max_size_time_sec * GST_SECOND, send_keyframe_request ? "true" : "false",
       encoder_key_interval_sec * 5);
 
@@ -964,6 +987,16 @@ splitmuxsink_split_by_keyframe (gboolean send_keyframe_request,
   g_free (dest_pattern);
   g_object_unref (sink);
 
+  enc = gst_bin_get_by_name (GST_BIN (pipeline), "enc");
+  fail_if (enc == NULL);
+  srcpad = gst_element_get_static_pad (enc, "src");
+  fail_if (srcpad == NULL);
+
+  gst_pad_add_probe (srcpad, GST_PAD_PROBE_TYPE_EVENT_UPSTREAM,
+      (GstPadProbeCallback) count_upstrea_fku, &upstream_fku_count, NULL);
+  gst_object_unref (srcpad);
+  gst_object_unref (enc);
+
   msg = run_pipeline (pipeline);
 
   if (GST_MESSAGE_TYPE (msg) == GST_MESSAGE_ERROR)
@@ -978,6 +1011,18 @@ splitmuxsink_split_by_keyframe (gboolean send_keyframe_request,
   fail_unless (count == expected_count,
       "Expected %d output files, got %d", expected_count, count);
 
+  if (!send_keyframe_request) {
+    expected_fku_count = 0;
+  } else {
+    expected_fku_count = count;
+  }
+
+  GST_INFO ("Upstream force keyunit event count %d", upstream_fku_count);
+
+  fail_unless (upstream_fku_count == expected_fku_count,
+      "Expected upstream force keyunit event count %d, got %d",
+      expected_fku_count, upstream_fku_count);
+
   in_pattern = g_build_filename (tmpdir, "out*.m4v", NULL);
   /* FIXME: Reverse playback works poorly with multiple video streams
    * in qtdemux (at least, maybe other demuxers) at the time this was
@@ -1035,17 +1080,21 @@ splitmuxsink_split_by_keyframe_timecode (gboolean send_keyframe_request,
   GstMessage *msg;
   GstElement *pipeline;
   GstElement *sink;
+  GstElement *enc;
+  GstPad *srcpad;
   gchar *pipeline_str;
   gchar *dest_pattern;
   guint count;
   guint expected_count;
   gchar *in_pattern;
+  guint upstream_fku_count = 0;
+  guint expected_fku_count;
 
   pipeline_str = g_strdup_printf ("splitmuxsink name=splitsink "
       "max-size-timecode=%s"
       " send-keyframe-requests=%s muxer=qtmux "
       "videotestsrc num-buffers=30 ! video/x-raw,width=80,height=64,framerate=5/1 "
-      "! videoconvert ! timecodestamper ! queue ! vp8enc keyframe-max-dist=%d ! splitsink.video ",
+      "! videoconvert ! timecodestamper ! queue ! vp8enc name=enc keyframe-max-dist=%d ! splitsink.video ",
       maxsize_timecode_string, send_keyframe_request ? "true" : "false",
       encoder_key_interval_sec ? encoder_key_interval_sec * 5 : 1);
 
@@ -1062,6 +1111,16 @@ splitmuxsink_split_by_keyframe_timecode (gboolean send_keyframe_request,
   g_free (dest_pattern);
   g_object_unref (sink);
 
+  enc = gst_bin_get_by_name (GST_BIN (pipeline), "enc");
+  fail_if (enc == NULL);
+  srcpad = gst_element_get_static_pad (enc, "src");
+  fail_if (srcpad == NULL);
+
+  gst_pad_add_probe (srcpad, GST_PAD_PROBE_TYPE_EVENT_UPSTREAM,
+      (GstPadProbeCallback) count_upstrea_fku, &upstream_fku_count, NULL);
+  gst_object_unref (srcpad);
+  gst_object_unref (enc);
+
   msg = run_pipeline (pipeline);
 
   if (GST_MESSAGE_TYPE (msg) == GST_MESSAGE_ERROR)
@@ -1077,6 +1136,18 @@ splitmuxsink_split_by_keyframe_timecode (gboolean send_keyframe_request,
   fail_unless (count == expected_count,
       "Expected %d output files, got %d", expected_count, count);
 
+  if (!send_keyframe_request) {
+    expected_fku_count = 0;
+  } else {
+    expected_fku_count = count;
+  }
+
+  GST_INFO ("Upstream force keyunit event count %d", upstream_fku_count);
+
+  fail_unless (upstream_fku_count == expected_fku_count,
+      "Expected upstream force keyunit event count %d, got %d",
+      expected_fku_count, upstream_fku_count);
+
   in_pattern = g_build_filename (tmpdir, "out*.m4v", NULL);
   /* FIXME: Reverse playback works poorly with multiple video streams
    * in qtdemux (at least, maybe other demuxers) at the time this was
@@ -1087,6 +1158,15 @@ splitmuxsink_split_by_keyframe_timecode (gboolean send_keyframe_request,
   g_free (in_pattern);
 }
 
+GST_START_TEST (test_splitmuxsink_without_keyframe_request_timecode)
+{
+  /* This encoding option is intended to produce keyframe per 1 second
+   * but splitmuxsink will split file per 2 second without keyframe request */
+  splitmuxsink_split_by_keyframe_timecode (FALSE, "00:00:02:00", 2, 1);
+}
+
+GST_END_TEST;
+
 GST_START_TEST (test_splitmuxsink_keyframe_request_timecode)
 {
   /* This encoding option is intended to produce keyframe per 1 second
@@ -1195,6 +1275,8 @@ splitmux_suite (void)
   }
 
   if (have_qtmux && have_vp8 && have_timecodestamper) {
+    tcase_add_test (tc_chain,
+        test_splitmuxsink_without_keyframe_request_timecode);
     tcase_add_test (tc_chain, test_splitmuxsink_keyframe_request_timecode);
     tcase_add_test (tc_chain,
         test_splitmuxsink_keyframe_request_timecode_trailing_small_segment);