[Bug 787922] splitmuxsink: add a "split at running time" action signal

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #3 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 360210:
 --> (https://bugzilla.gnome.org/review?bug=787922&attachment=360210)

Thanks for the patch, this looks useful :)

::: gst/multifile/gstsplitmuxsink.c
@@ +299,3 @@
+  /**
+   * GstSplitMuxSink::split-at-running-time:
+   * @splitmux: An action signal that takes a GstClockTime as an argument.
When called by the user, the video file is split (and a new one begins) as soon
as the pipeline reaches or exceeds the running time specified. An argument of 0
splits the file immediately.

Here describe the arguments to the signal, shortly (and this is also why names
are needed)

> @splitmux: the splitmuxsink
> @running_time: running time to split at

And then write some actual documentation in multiple lines saying what this
does. It tries to split at that running time next. How does it interact with
automatically selected split points (by key-frame, duration, etc)? Maybe there
should also be a property to only ever split based on the signal and never
automatically

@@ +302,3 @@
+   * 
+   *
+   * Returns: naught.

If it returns nothing, just don't put a "Returns: " line here

Also add a "Since: 1.14" line

@@ +333,2 @@
   GST_OBJECT_FLAG_SET (splitmux, GST_ELEMENT_FLAG_SINK);
+  splitmux->time_to_split = GST_CLOCK_TIME_NONE;

This should also be reset in the state change from PAUSED to READY

@@ +2326,3 @@
+
+static void
+set_time_to_split (GstSplitMuxSink * splitmux, GstClockTime _time_to_split)

No underscore in front of variables

::: gst/multifile/gstsplitmuxsink.h
@@ +174,3 @@
+  
+  /* actions */
+  void     (*split_at_running_time)   (GstSplitMuxSink *, GstClockTime);

We try to give names to the arguments here. While not required it helps
understanding what the function pointers actually do.

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