After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 732372 - matroskademux: send gap events instead of doing 0.10-style segment tricks
matroskademux: send gap events instead of doing 0.10-style segment tricks
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.x
Other Linux
: Normal major
: 1.3.91
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-06-28 00:07 UTC by Nicola
Modified: 2014-07-04 12:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
matroskademux: update segment offset to match missing frames (1.97 KB, patch)
2014-07-01 10:38 UTC, Vincent Penquerc'h
rejected Details | Review
matroskademux: send gap events instead of segment tricks (2.69 KB, patch)
2014-07-01 14:16 UTC, Vincent Penquerc'h
committed Details | Review

Description Nicola 2014-06-28 00:07:00 UTC
please take a look at this file:

http://195.250.34.59/temp/bug_mkv10.mkv

this is a streamable mkv

try:

gst-launch-1.0 filesrc location=... ! matroskademux ! matroskamux ! filesink location=...

you'll get a file with 25 seconds duration, this is not correct

if you use gstreamer 0.10 you'll get a file with 50 seconds duration, this is right,

the last overlay date and time is 01:42:14 in the file produced with 0.10 (correct) and 01:38:10 in the file produced with 1.0

the file has a lot of gaps and so many new segment events are generated,

the regression seems in the demuxer
Comment 1 Vincent Penquerc'h 2014-06-30 15:51:37 UTC
Why is 50 seconds correct, and not 4:44 instead ?

I have made a change to send gap events matching the, well, gaps in timestamps, and the resulting playback plays in about 4:48, and the timing of the on-screen timestamps seem largely right.
Comment 2 Nicola 2014-06-30 15:59:04 UTC
50 sec is the effective stream time, in 0.10 when there is a gap the video is not frozen on the screen
Comment 3 Vincent Penquerc'h 2014-07-01 10:38:52 UTC
Created attachment 279664 [details] [review]
matroskademux: update segment offset to match missing frames

Assuming the desired behavior is to skip the gaps, rather than match the input timestamps, this patch restores the 0.10 timing.

Also needs the patch from https://bugzilla.gnome.org/show_bug.cgi?id=732540 to avoid suprious warnings about slow machine resulting from the jumps between incoming timestamps.
Comment 4 Sebastian Dröge (slomo) 2014-07-01 11:16:47 UTC
I think the current behaviour is correct. If there are gaps in the timestamps in the container we should not just ignore them but use the timestamps as they are in the container. Otherwise we could also easily get a/v sync wrong.

Or did I misunderstand what this is about?
Comment 5 Vincent Penquerc'h 2014-07-01 11:29:32 UTC
I would tend to think the current behavior is correct too, though I'm not too certain. I asked why not in comment 1, so unless the reporter explains why 50 seconds is right insead of 4:44, feel free to ignore the patch.
Comment 6 Nicola 2014-07-01 11:32:06 UTC
thanks works better with the patch even if the file produced with 0.10 still show more frames than the one produced with 1.0

I'll try to explain my use case:

- I save videos from source that can be connected with low bandwidth (the attached file is an extreme one simulated with curl --limit-rate)
- the camera skip some frame and restart to send from the next keyframe when the client is not fast enough to receive from network so gaps are created

when I play the live stream the video is frozen on gaps, this is correct the camera is sending nothing

however when I play the saved file I don't want the player show a frozen video on gap, the behaviour in 0.10 to play the next period seems ok (or at least is ok for my use case). In general if I have a file on disk with timestamp gaps the file should skip the gaps and start to play normally from the new timestamp without show a frozen images until the running time match the new timestamp, if you have a file with the first timestamp setted as 60 seconds instead of 0 it should start to play immediatly and not after 60 seconds
Comment 7 Nicola 2014-07-01 11:33:51 UTC
Vincent I think 50 seconds is correct and not 4:44 since you have a file playable for 50 seconds
Comment 8 Sebastian Dröge (slomo) 2014-07-01 12:47:10 UTC
You have to remove the gaps while recording/muxing your file then. The file actually is 4:44 according to the timestamps, but contains timestamp gaps. Just ignoring the gaps would be wrong.
Comment 9 Nicola 2014-07-01 12:54:02 UTC
ok, but at least push the fix that make the file playable for 4:44, with actual git master you'll get a 25 seconds file,

"""
I have made a change to send gap events matching the, well, gaps in timestamps,
and the resulting playback plays in about 4:48, and the timing of the on-screen
timestamps seem largely right.
"""

this change is not in git master
Comment 10 Sebastian Dröge (slomo) 2014-07-01 12:56:35 UTC
Which patch is that?
Comment 11 Nicola 2014-07-01 13:05:00 UTC
I think Vincent has this patch locally,

with actual git master and 1.x if you remux the file you'll get a 25 seconds file (in 0.10 a 50 seconds file) 

if you play the file with 1.0 you'll see less frame than with 0.10,

for example using this pipeline:

gst-launch-1.0 filesrc location= /tmp/bug_mkv10.mkv ! matroskademux ! avdec_h264 ! xvimagesink

you can observe the video timestamp overlay that print:

01:37:39
01:37:40
01:37:41
01:37:42
01:37:43
...
... really quick play
...
01:37:49

in 0.10 you'll see:

01:37:42
01:37:43
... gap
01:37:45
01:37:46
01:37:47
01:37:48
Comment 12 Sebastian Dröge (slomo) 2014-07-01 13:08:21 UTC
Vincent?
Comment 13 Vincent Penquerc'h 2014-07-01 13:09:38 UTC
Yes, it was my first try, which led me to my first question above. I was sending gap events when gaps were detected. I don't actually have that patch anymore, I will rewrite it after lunchtime and post here. It was a simple one.
Comment 14 Sebastian Dröge (slomo) 2014-07-01 13:11:36 UTC
Thanks, makes sense now :)

So basically what it should do

1:00
1:01
gap events for 19 seconds
1:20
1:21
1:22
Comment 15 Vincent Penquerc'h 2014-07-01 14:16:59 UTC
Created attachment 279690 [details] [review]
matroskademux: send gap events instead of segment tricks

time gst-launch-1.0 [...] takes 1:48  with this. On screen timestamps roughly match real time. Hard to say if it's off by some small amount at the gaps...
Comment 16 Sebastian Dröge (slomo) 2014-07-01 14:24:55 UTC
Comment on attachment 279690 [details] [review]
matroskademux: send gap events instead of segment tricks

Makes sense, why did this do any segment stuff at all in 1.0? Looks like something missed while porting
Comment 17 Tim-Philipp Müller 2014-07-01 14:26:00 UTC
The gap events or absence thereof shouldn't affect timing of the buffers for what it's worth, they're just grease to keep the pipeline ticking along (e.g. multiqueue or muxers or whatever).
Comment 18 Sebastian Dröge (slomo) 2014-07-01 14:39:23 UTC
Yes, but fiddling with the segment will :)
Comment 19 Nicola 2014-07-01 15:10:25 UTC
the patch look good to me, thanks!