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 761086 - splitmuxsrc: use the max ts of all streams as the offset to align the next stream with
splitmuxsrc: use the max ts of all streams as the offset to align the next st...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.11.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-01-25 14:51 UTC by George Kiagiadakis
Modified: 2017-02-27 11:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
splitmuxsrc: use the max ts of all streams as the offset to align the next stream with (1.42 KB, patch)
2016-01-25 14:51 UTC, George Kiagiadakis
none Details | Review
splitmuxpartreader: identify sparse streams (1.13 KB, patch)
2017-02-23 10:15 UTC, George Kiagiadakis
accepted-commit_now Details | Review
splitmuxpartreader: ignore sparse streams when calculating the end offset of a part (1.21 KB, patch)
2017-02-23 10:15 UTC, George Kiagiadakis
accepted-commit_now Details | Review
tests: splitmux: add unit test for content with sparse streams (10.43 KB, patch)
2017-02-23 10:16 UTC, George Kiagiadakis
accepted-commit_now Details | Review

Description George Kiagiadakis 2016-01-25 14:51:04 UTC
Created attachment 319684 [details] [review]
splitmuxsrc: use the max ts of all streams as the offset to align the next stream with

Splitmuxsrc currently finds and uses the minimum timestamp of all streams (audio/video/subtitle) as the first timestamp for the next part, but this is incorrect and gets noticed when you have subtitles, which are sparse. (In audio/video cases it can easily go unnoticed because the ending timestamps are very close together)

So for example if the current part ends at t=5s and the last subtitle was at t=1s, then the next part's timestamps start from t=1s and you lose 4 seconds of data in the output.

The attached patch fixes the issue.
Comment 1 Jan Schmidt 2016-09-28 15:52:30 UTC
I only just noticed this bug. Using the max of all timestamps is not correct, and pretty much guarantees that there'll be a gap in the audio or video timelines, which are supposed to be seamless.
Comment 2 George Kiagiadakis 2017-02-21 15:01:36 UTC
I was just looking at this again. You are right, it makes sense to use the min of all timestamps, but there should be a way to exclude sparse streams from the calculation.
Comment 3 Jan Schmidt 2017-02-22 05:20:16 UTC
I agree. Basing it on the stream-start event sparse flag should work. I think that's widely enough implemented.
Comment 4 George Kiagiadakis 2017-02-23 10:15:21 UTC
Created attachment 346554 [details] [review]
splitmuxpartreader: identify sparse streams
Comment 5 George Kiagiadakis 2017-02-23 10:15:48 UTC
Created attachment 346555 [details] [review]
splitmuxpartreader: ignore sparse streams when calculating the end offset of a part
Comment 6 George Kiagiadakis 2017-02-23 10:16:13 UTC
Created attachment 346556 [details] [review]
tests: splitmux: add unit test for content with sparse streams
Comment 7 Jan Schmidt 2017-02-24 08:18:44 UTC
Review of attachment 346554 [details] [review]:

Looks good
Comment 8 Jan Schmidt 2017-02-24 08:19:33 UTC
Review of attachment 346555 [details] [review]:

Looks good
Comment 9 Jan Schmidt 2017-02-24 08:21:41 UTC
Review of attachment 346556 [details] [review]:

Awesome, thanks!
Comment 10 George Kiagiadakis 2017-02-27 11:11:01 UTC
commit e6bd2a5c180958f74d58549ac8786e8e5584bd37
Author: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date:   Thu Feb 23 12:11:15 2017 +0200

    tests: splitmux: add unit test for content with sparse streams
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761086

commit 9b845133377275e0e218f878d739b3869efc7510
Author: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date:   Wed Feb 22 11:23:19 2017 +0200

    splitmuxpartreader: ignore sparse streams when calculating the end offset of a part
    
    A sparse stream's ending timestamp can be considerably smaller
    than the ending timestamps of the other streams, which can lead
    to skipping considerable time from the next part.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761086

commit 99728792cde38ac6cd2cc30d84d5aede84b50c04
Author: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date:   Wed Feb 22 11:21:06 2017 +0200

    splitmuxpartreader: identify sparse streams