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 700264 - qtdemux: ignores first editlist
qtdemux: ignores first editlist
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal major
: 1.1.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-05-14 00:44 UTC by Matej Knopp
Modified: 2013-08-12 11:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.10 KB, patch)
2013-05-14 00:46 UTC, Matej Knopp
none Details | Review
Patch (4.54 KB, patch)
2013-07-06 15:28 UTC, Matej Knopp
reviewed Details | Review
Updated patch for current master (4.42 KB, patch)
2013-07-14 12:12 UTC, Matej Knopp
committed Details | Review
Patch to offset samples instead of buffers (3.00 KB, patch)
2013-07-31 08:45 UTC, Matej Knopp
committed Details | Review

Description Matej Knopp 2013-05-14 00:44:05 UTC
The first editlist segment can be used to offset samples. Qtdemux currently ignores it, which can results in synchronization issues. I.e. subtitles appearing right from the beginning instead of their proper time.
Comment 1 Matej Knopp 2013-05-14 00:46:35 UTC
Created attachment 244125 [details] [review]
Patch

Simple patch that fixes it, at least for my test files. This is just a quick fix though, probably not the best solution. Would be nice if someone more familiar with qtdemux could look at it.
Comment 2 Tim-Philipp Müller 2013-05-14 08:54:25 UTC
Could you make a/your test file available by any chance?
Comment 3 Matej Knopp 2013-05-14 12:03:34 UTC
Here's a sample file

https://s3.amazonaws.com/MatejK/testelst1.mp4

the subtitle track should start at ~ 30s, with gstreamer it is displayed right from the beginning.
Comment 4 Matej Knopp 2013-07-06 15:28:35 UTC
Created attachment 248527 [details] [review]
Patch

Better patch - offsets samples only by minimal amount (to maintain relative synchronization) and adjusts file duration
Comment 5 Matej Knopp 2013-07-06 15:32:21 UTC
Another sample file

https://s3.amazonaws.com/MatejK/fellowshipoftherings_fs.mov

In this case both streams have edit list with offset of 3 seconds. This results in gstreamer reporting file length that is 3 second longer than actual data.
Comment 6 Sebastian Dröge (slomo) 2013-07-09 09:09:01 UTC
(In reply to comment #4)
> Created an attachment (id=248527) [details] [review]
> Patch
> 
> Better patch - offsets samples only by minimal amount (to maintain relative
> synchronization) and adjusts file duration

It shouldn't really make a difference by how much you offset the samples. What difference does it make?

Otherwise this looks good
Comment 7 Matej Knopp 2013-07-09 09:18:08 UTC
Well, in the test file all samples then start from 3 seconds, so there is a 3 second gap at the beginning. Is that really necessary? Or maybe the segment should be adjusted instead?
Comment 8 Sebastian Dröge (slomo) 2013-07-09 10:08:40 UTC
What is the semantic meaning of the 3 seconds here? What should it cause the player to do? Why would anybody put a 3s gap in the beginning if he doesn't expect players to wait for 3 seconds?

Putting it into the segment is also an option, yes.
Comment 9 Matej Knopp 2013-07-09 10:17:41 UTC
In this particular file (fellowshipoftherings_fs.mov)  there is a track that acts like a background image. So when playing in quicktime, for first 3 second only the background image is shown. 

GStreamer however ignores the background image track (being too short), so  offseting samples for both tracks there would just be gap. 

I think in this situation it might make sense to just start from PTS 0 in gstreamer and adjust duration to reflect the actual data length. I don't see any point for the gap, gstreamer doesn't show the background picture (doesn't even reveal the track)

Other players (not quicktime) do various things with the file, including crashing and not playing at all.

If we just change the segment that still leaves the problem with duration being reported incorrect, doesn't it?
Comment 10 Sebastian Dröge (slomo) 2013-07-09 10:27:31 UTC
(In reply to comment #9)
> In this particular file (fellowshipoftherings_fs.mov)  there is a track that
> acts like a background image. So when playing in quicktime, for first 3 second
> only the background image is shown. 
> 
> GStreamer however ignores the background image track (being too short), so 
> offseting samples for both tracks there would just be gap. 
> 
> I think in this situation it might make sense to just start from PTS 0 in
> gstreamer and adjust duration to reflect the actual data length. I don't see
> any point for the gap, gstreamer doesn't show the background picture (doesn't
> even reveal the track)

Agreed but what about situations where you have several container files, and they should be played one after another? The next one having editlists start at the end position of the previous one? Does that make sense?

> If we just change the segment that still leaves the problem with duration being
> reported incorrect, doesn't it?

That can be fixed ;) But offsetting the timestamps makes sense too, I don't have a favorite solution.
Comment 11 Matej Knopp 2013-07-09 10:51:15 UTC
Would that still be done with the first edit list (that has mediatime == -1)? Do you have any files like that? I think this solution is better than what's done currently (ignoring the elst completely), but if you have any files this doesn't work with I can look at it.
Comment 12 Sebastian Dröge (slomo) 2013-07-09 10:57:33 UTC
I don't, just thinking out loud :) I agree that your solution is probably the best for now, but can you confirm that this still works with DASH streams (using fragmented mp4) for multiple segments?
Comment 13 Matej Knopp 2013-07-09 11:09:44 UTC
I don't think I can, I don't have any stream to test it with and I know next to nothing about DASH :). However if DASH does rely on the edit list to offset sample timestamps in any way, how could it work with current gstreamer at all?
Comment 14 Matej Knopp 2013-07-14 12:12:48 UTC
Created attachment 249117 [details] [review]
Updated patch for current master
Comment 15 Sebastian Dröge (slomo) 2013-07-15 08:00:08 UTC
I don't know much about DASH either, but you're right... it wouldn't have worked before then ;)

commit ca32442f86f38b5fe4b97c38af5f56b0bb847f3f
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Sat Jul 6 17:20:49 2013 +0200

    qtdemux: offset samples according to edit list
    
    https://bugzilla.gnome.org/show_bug.cgi?id=700264
Comment 16 Matej Knopp 2013-07-31 08:45:03 UTC
Created attachment 250522 [details] [review]
Patch to offset samples instead of buffers

So, the current approach where buffers are offset is not ideal, as during seek and loop current time is compared to sample times. Patch solves this by offsetting samples instead of output buffers.
Comment 17 Sebastian Dröge (slomo) 2013-08-12 11:49:38 UTC
commit 2269ac8f284d3401b9ca4e8524405e7ac6e8b964
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Wed Jul 31 10:42:07 2013 +0200

    qtdemux: elst should offset samples instead of buffers
    
    The current approach where buffers are offset is not ideal, as during seek
    and loop current time is compared to sample times.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=700264