[Bug 774209] splitmuxsink: Add option for timecode-based split

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=774209

Jan Schmidt <thaytan at noraisin.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #340050|none                        |needs-work
             status|                            |

--- Comment #4 from Jan Schmidt <thaytan at noraisin.net> ---
Review of attachment 340050:
 --> (https://bugzilla.gnome.org/review?bug=774209&attachment=340050)

I think this is not quite ready. Also, I'm working on a general rework of the
way splitmuxsink handles buffering to make everything more deterministic. It
might be worth holding off on reworking this patch until that's done.

::: gst/multifile/gstsplitmuxsink.c
@@ +668,3 @@
+
+  parts = g_strsplit (splitmux->threshold_timecode_str, ":", 4);
+  if (!parts || parts[3] == NULL) {

You shouldn't assume the array is large enough for this to be safe. Use
g_strv_length() to check the number of entries.

@@ +728,3 @@
+  GST_DEBUG_OBJECT (splitmux, "Next max TC time: %" GST_TIME_FORMAT,
+      GST_TIME_ARGS (next_max_tc_time));
+  gst_video_time_code_free (target_tc);

This code seems like functionality that should be in the timecode manipulation
library API?

@@ +1099,3 @@
+                  queued_time > splitmux->threshold_time) ||
+              (splitmux->next_max_tc_time != GST_CLOCK_TIME_NONE &&
+                  tc_time > splitmux->next_max_tc_time + 5 * GST_USECOND)))) {

What's the 5 microsecond magic number about?

@@ +1679,3 @@
   if (GST_PAD_PAD_TEMPLATE (pad) &&
+      g_str_equal (GST_PAD_TEMPLATE_NAME_TEMPLATE (GST_PAD_PAD_TEMPLATE
+              (pad)), "video"))

Why are there whitespace changes?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.