GNOME Bugzilla – Bug 731351
x264enc: Shift PTS and DTS with bframes
Last modified: 2015-06-12 23:37:38 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.
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 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
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.
Erm. How is this supposed not to result in A/V sync issues? You can't just shift one stream timestamps.
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).
Link to more elaborate description of current DTS handling problems, since it's somewhat relevant https://bugzilla.gnome.org/show_bug.cgi?id=740575
Do we need to fix something here before 1.6?
Perhaps this should be reverted until we have a more generic solution?
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.
I think we'll try to get bug #740575 in for 1.6.
That patch for this is actually attached to bug #740575.
This is now fix, see bug #740575 for details.