GNOME Bugzilla – Bug 721422
Negative pad offsets don't work
Last modified: 2014-01-15 10:26:50 UTC
Created attachment 265243 [details] Test case for the bug Even though the pad offset API takes a gint64, GstSegment.base is a guint64 so negative pad offsets have weird results. A test program is attached, just run it as: ./testsave file:///path/to/some/file.mkv outputdir The program will split the mkv file into several and if it works properly each file will have the proper timestamps (start from 0s). With the current pad offset code this doesn't happen.
For this we probably have to add yet another offset field to GstSegment, this time a signed one. However adding a field there now is arguably breaking ABI as every code that calculated with the GstSegment fields directly instead of the gst_segment_* functions won't work properly afterwards. One corner case for negative pad offsets is btw that the overall calculations yield a negative time. In that case I think it's safe to say that it's outside the segment?
In theory it should work: 1) positive offsets adjust the base time 2) negative offsets adjust the start/stop time and possibly base time if we can We only do 1) currently. After implementing 2) this example still doesn't work because of matroskamux. It converts the buffer timestamps to running time twice, first in gst_collect_pads_clip_running_time() and then in gst_matroska_mux_handle_buffer().
(In reply to comment #2) > In theory it should work: > > 1) positive offsets adjust the base time > 2) negative offsets adjust the start/stop time and possibly base time if we > can > > We only do 1) currently. 2) sounds like a good plan for this, yes > After implementing 2) this example still doesn't work because of matroskamux. > It converts the buffer timestamps to running time twice, first in > gst_collect_pads_clip_running_time() and then in > gst_matroska_mux_handle_buffer(). Indeed, but gst_collect_pads_clip_running_time() only adjusts the PTS and DTS, not the duration. Also qtmux has the same problem. This warrants another bug...
first part: commit 7f8c4dceb449b5fbb8bd8a8cfc58d218d2e0cd00 Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Jan 8 11:28:04 2014 +0100 Revert "matroskamux: Use the running time for container timestamps, not buffer timestamps" This reverts commit b3aa8755fe07639f22e4104f4932d769d6c9075a. We are already using the running-time because they were placed on the buffers with gst_collect_pads_clip_running_time(). Arguably it would be better to not modify the incomming buffers but collectpads seems to want to use absolute timestamps from the buffers for finding the best buffer (this can be changed with a custom compare function..).
commit 6d3fc584d5573ae99c090bf3d169d4edee6be89a Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Jan 8 14:57:04 2014 +0100 pad: use new segment offset method to apply the offset Fixes https://bugzilla.gnome.org/show_bug.cgi?id=721422 commit dc0176e4a072888aec029f008e6ab4357ad09e96 Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Jan 8 14:54:47 2014 +0100 segment: add method to offset the segment running-time Add a method that can apply an offset to the calculated running-time of a segment.