[Bug 769664] splitmuxsink: Add option to split at exactly max-size-time

[GStreamer (GNOME Bugzilla)]

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

Jan Schmidt <thaytan at noraisin.net> changed:

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

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

Thanks for the patch! Comments inline.

::: gst/multifile/gstsplitmuxsink.c
@@ +215,3 @@
+          "Split at exactly max-size-time ns. "
+          "Needs max-size-bytes to be 0 in order to be effective.",
+          DEFAULT_EXACT_TIME, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));

I'm not sure about the naming here. It should be something that indicates
splitmuxsink will request split points. "Split at exactly max-size-time" might
not be possible if upstream ignores keyframe requests, or implements them
incorrectly.

@@ +844,3 @@
   /* Store the overflow parameters as the basis for the next fragment */
+  splitmux->mux_start_time =
+      splitmux->muxed_out_time + splitmux->last_frame_duration;

Since splitmux->last_frame_duration might be GST_CLOCK_STIME_NONE if the buffer
has no provided duration, we can only add it if valid.

@@ +948,1 @@


I'm not sure about removing the equality check here.

@@ +1711,3 @@
+            "Could not request a keyframe. Files may not split at the exact
location they should");
+      break;
+    }

The output of splitmuxsink should be independent of repeated pausing/playing.
This will request a new keyframe every time, which isn't right. If it's needed
in order to generate the initial keyframe request, that should perhaps happen
when the first data is released to the muxer.

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

Thanks for the patch! Comments inline.

::: gst/multifile/gstsplitmuxsink.c
@@ +215,3 @@
+          "Split at exactly max-size-time ns. "
+          "Needs max-size-bytes to be 0 in order to be effective.",
+          DEFAULT_EXACT_TIME, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));

I'm not sure about the naming here. It should be something that indicates
splitmuxsink will request split points. "Split at exactly max-size-time" might
not be possible if upstream ignores keyframe requests, or implements them
incorrectly.

@@ +844,3 @@
   /* Store the overflow parameters as the basis for the next fragment */
+  splitmux->mux_start_time =
+      splitmux->muxed_out_time + splitmux->last_frame_duration;

Since splitmux->last_frame_duration might be GST_CLOCK_STIME_NONE if the buffer
has no provided duration, we can only add it if valid.

@@ +948,1 @@


I'm not sure about removing the equality check here.

@@ +1711,3 @@
+            "Could not request a keyframe. Files may not split at the exact
location they should");
+      break;
+    }

The output of splitmuxsink should be independent of repeated pausing/playing.
This will request a new keyframe every time, which isn't right. If it's needed
in order to generate the initial keyframe request, that should perhaps happen
when the first data is released to the muxer.

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