GNOME Bugzilla – Bug 752106
flacparse: makes up bogus DTS for audio after seeking
Last modified: 2015-09-07 11:43:46 UTC
after seek(jump), some file pts faster than dts. we need to adjust at least same. Fix for playback after seek. Recalculate dts if dts exceeds pts.
Created attachment 307051 [details] [review] Recalculate dts if dts exceeds pts
I wonder if there isn't a bigger problem here. Why do audio packets get their DTS set in the first place (by baseparse, presumably). I don't think that should happen?
FLAC shouldn't have any DTS at all, or at least DTS should always be the same as PTS.
Is this a regression btw? Which GStreamer version are you testing?
There is/was also an issue (possibly a bug report too) where baseparse puts dts on outgoing mp3 packets in mpegaudioparse even though tsdemux only ever puts pts on the input packets. I'm assuming this is all related, but it might not be of course.
Whenever a new buffer is arrived, its being parsed in the function “gst_flac_parse_parse_frame()” defined in file gstflacparse.c This function is doing following things: 1. It is calculating buffer PTS value every time based on sample number and sample rate, which is correct GST_BUFFER_TIMESTAMP (buffer) = gst_util_uint64_scale (flacparse->sample_number, flacparse->block_size * GST_SECOND, flacparse->samplerate); 2. Here we are doing nothing to recalculate the DTS of the same buffer. DTS value is maintained by the base parse. 3. When that buffer is pushed by API “gst_pad_push()” , for certain streams, PTS is correct value but DTS is getting ahead of PTS. Which is an error. 4. This patch is an error check, under no conditions, DTS value should exceeds the PTS value. 5. If this patch is not applied then for certain streams, we observe glitches/no sound after seek as, DTS value is getting exceeded by PTS.
So maybe flacparse should just unset DTS?
yes, the flacparser needs to assign DTS equal to PTS, for the first buffer after the seek. For the normal contineous playback, the issue is not observed.
is there are any progress? ^^
Can you provide some more details? What kind of files trigger this? FLAC? FLAC into some container? Does it always happen after a seek?
flac audio only file. I downloaded it in commercial music streaming service. After seek, the PTS value wrong for around 4 secs. Dear Rajesh Singh, could you share a problem file?
I also have the impression there is some bogus code in baseparse. Somewhere in the code, we have: if (next_pts >= parse->priv->next_dts) parse->priv->next_pts = next_pts; But if it wasn't bigger, it left with an old (and probably worst) value.
Problematic/test stream file size is 118 MB, Please share me, the efficient way to share the large problematic/test stream.
You can put on your favorite online file sharing service/host and put the link here. Services that won't require login to download the file are preferred.
Nicolas, do you have suggestions what to change in the code to make it less weird?
Dear Thiago Sousa, Can i divide the file in parts and send you over your email?
Not ideal but if that is all you can do, please do it. Can I put the file somewhere public after receiving so other developers can check it, too?
Why is this a blocker? Is it a regression? Arguably the audio parsers should never set DTS and only set PTS.
Dear Thiago Sousa, I tried to send you, test stream over mail but mail delivery is getting failed for mail id thiagossantos@gmail.com. "Delivery failed: thiagossantos@gmail.com Remote host said[Response Message]: 552-5.7.0 This message was blocked because its content presents a potential 552-5.7.0 security issue"
Found a file I can reproduce this with, it yields this (between flacparse and flacdec): ... dts: 0:00:05.146122448, pts:0:00:05.146122448 Seeking to 60.0 seconds.. dts: 0:01:00.104489796, pts:0:00:59.977142857 ... and the dts/pts diff remains for the rest. This happens with 1.4 as well though.
Dear Tim, Similar flac streams, I also found which I am unable to share due to huge size. While doing seek near the end of duration, i observed difference between DTS and PTS even more than 5 seconds. Above condition can be avoided, by assigning DTS equal to PTS. This assignment of DTS equal to PTS, will be called only once after seek, if they differ.
You're right indeed, the difference is larger towards the end: Seeking to 200.0 seconds.. dts: 0:03:25.616326530, pts:0:03:19.993469387, duration: 0:00:00.026122449
Created attachment 310678 [details] [review] flacparse: set both pts and dts so baseparse doesn't make up wrong dts after a seek
Created attachment 310679 [details] [review] wavpackparse: set both pts and dts so baseparse doesn't make up wrong dts after seeks
Comment on attachment 307051 [details] [review] Recalculate dts if dts exceeds pts This seems more like a workaround, better fix the actual problem. We know what the DTS should be here (either not set or same as PTS).
I assume you checked that none of the other audio parsers, and none of the video parsers are needed to be fixed the same way?
It affects parsers for framed formats where timestamps are created based on information in the headers (like sample offset or frame offset). I've only fixed up audioparsers for now. Will have a look at the other GstBaseParse sub-classes later. In any case, it's not a regression anyway.commit fcdb79ef7b7ff55c8a64573cc991fca04dc7e590 Author: Tim-Philipp Müller <tim@centricular.com> Date: Fri Sep 4 19:45:37 2015 +0100 wavpackparse: set both pts and dts so baseparse doesn't make up wrong dts after seeks https://bugzilla.gnome.org/show_bug.cgi?id=752106 commit 0d88f271084cce54de38907a618368deed843ee8 Author: Tim-Philipp Müller <tim@centricular.com> Date: Fri Sep 4 19:34:41 2015 +0100 flacparse: set both pts and dts so baseparse doesn't make up wrong dts after a seek flac contains the sample offset in the frame header, so after a seek without index flacparse will know the exact position we landed on and timestamp buffers accordingly. It only set the pts though, which means the baseparse-set dts which was set to the seek position prevails, and since the seek was based on an estimate, there's likely a discrepancy between where we wanted to land and where we did land, so from here on that dts/pts difference will be maintained, with dts possibly multiple seconds ahead of pts, which is just wrong. The easiest way to fix this is to just set both pts and dts based on the sample offset, but perhaps parsed audio should just not have dts set at all. https://bugzilla.gnome.org/show_bug.cgi?id=752106
asfparse: probably could use some changes, but not even asfdemux or asfmux do any dts-handling yet, so might just as well leave it to when someone's interested in this. opusparse: just makes up timestamps from 0 it seems (see bug #754669), probably not syncable anyway, so no issue there for this bug. ivfparse: not syncable, so seeking will be based on index or scanning from good position, so videoparsers: vc1parse, h264parse, h265parse do some things, but they all lack explicit dts handling anyway and have well-known deficiencies in that area, so let's not touch them.