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 731351 - x264enc: Shift PTS and DTS with bframes
x264enc: Shift PTS and DTS with bframes
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
git master
Other Linux
: Normal blocker
: 1.5.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 740575
Blocks:
 
 
Reported: 2014-06-07 00:05 UTC by Nicolas Dufresne (ndufresne)
Modified: 2015-06-12 23:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] x264enc: Shift both PTS and DTS to ensure positive timestamp (3.20 KB, patch)
2014-06-07 00:26 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2014-06-07 00:05:27 UTC
In presence of b-frames, x264 will produce negative DTS, which isn't supported by GStreamer. In order to work-around this, we shift the DTS forward to make it non-zero. This has the impact that we don't respect that rule where PTS >= DTS. While most element only uses that for ordering, it's not to much of an issue.

The problem though is present in FLV format. The container stores the DTS and a positive delta (PTS - DST). As PTS >= DTS is not respected, we are unable to correctly calculate the required delta.

The solution I propose is to shift both DTS and PTS accordingly. This way they stay aligned to their original. This expose another bug in the encoder, which says that on keyframe PTS == DTS, this is not true for H264, but I that being a just a warning, I would not mind too much right now.
Comment 1 Nicolas Dufresne (ndufresne) 2014-06-07 00:26:05 UTC
Created attachment 278067 [details] [review]
[PATCH] x264enc: Shift both PTS and DTS to ensure positive timestamp


Currently we only shift DTS to compensate that we don't support negative
timestamp. This cause a problem that PTS is no longer >= DTS and may
make muxers live much harder. Instead, shift both PTS/DTS forward. Also
remove all the hack to handle this which seems the result of thinking libx264
is bugged.

https://bugzilla.gnome.org/show_bug.cgi?id=73135
---
 ext/x264/gstx264enc.c | 26 +++++++++-----------------
 ext/x264/gstx264enc.h |  2 +-
 2 files changed, 10 insertions(+), 18 deletions(-)
Comment 2 Nicolas Dufresne (ndufresne) 2014-07-25 19:47:51 UTC
Comment on attachment 278067 [details] [review]
[PATCH] x264enc: Shift both PTS and DTS to ensure positive timestamp

commit 698714fc9781c614ea5d65744508d5db73f16066
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Fri Jun 6 20:23:15 2014 -0400

    x264enc: Shift both PTS and DTS to ensure positive timestamp
    
    Currently we only shift DTS to compensate that we don't support negative
    timestamp. This cause a problem that PTS is no longer >= DTS and may
    make muxers live much harder. Instead, shift both PTS/DTS forward. Also
    remove all the hack to handle this which seems the result of thinking libx264
    is bugged.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=731351
Comment 3 Nicolas Dufresne (ndufresne) 2014-07-25 19:50:48 UTC
This might not be perfect, in the sense that it generally remains unclear if segment should fit PTS or DTS. In this case the segment remains too large, fitting both. Never the less, I merged it because it removes a hack that isn't always predictable.
Comment 4 Matej Knopp 2014-11-22 22:11:15 UTC
Erm. How is this supposed not to result in A/V sync issues? You can't just shift one stream timestamps.
Comment 5 Nicolas Dufresne (ndufresne) 2014-11-22 22:55:56 UTC
Previously we where assuming x264 was wrong, that PTS == DTS on key frame. So
we where shifting DTS up to "fix" it, that was just more wrong. Now, the
problem is that if first input has PTS 0, then it means that first DTS will
most likely be negative. We don't support this in GStreamer.

Now, as I mention, this forward shift need to be compensated somewhere, just
didn't have time to handle it yet. I need to review the entire segment clipping
code to make sure everything sane (my first look told me there would be few
bugs along the way).
Comment 6 Matej Knopp 2014-11-23 16:11:19 UTC
Link to more elaborate description of current DTS handling problems, since it's somewhat relevant
https://bugzilla.gnome.org/show_bug.cgi?id=740575
Comment 7 Sebastian Dröge (slomo) 2015-03-15 15:22:45 UTC
Do we need to fix something here before 1.6?
Comment 8 Tim-Philipp Müller 2015-03-15 15:34:19 UTC
Perhaps this should be reverted until we have a more generic solution?
Comment 9 Matej Knopp 2015-03-15 15:38:33 UTC
Well, last time I have checked x264enc in master still shifts timestamp without adjusting segment start so you have one or two frames A/V sync delay.
Comment 10 Tim-Philipp Müller 2015-03-15 15:47:27 UTC
I think we'll try to get bug #740575 in for 1.6.
Comment 11 Nicolas Dufresne (ndufresne) 2015-03-15 16:32:11 UTC
That patch for this is actually attached to bug #740575.
Comment 12 Nicolas Dufresne (ndufresne) 2015-06-12 23:37:30 UTC
This is now fix, see bug #740575 for details.