GNOME Bugzilla – Bug 643591
ffmpegdec: invalid timestamp being used for next timestamp calculation
Last modified: 2011-08-26 10:05:39 UTC
Created attachment 182178 [details] [review] proposed patch When only invalid input timestamps arrive, output timestamps are based on next_out, which is initialized as GST_CLOCK_TIME_NONE. This is usually not inside the current segment. clip_audio_buffer will return TRUE for the first buffer (which has an invalid timestamp), but will return FALSE for the next buffers, since they use now next_out (with a value of the last timestamp plus duration) as timestamp. One of the bugs here is to set next_out as timestamp plus duration when timestamp is invalid. One solution is to use next_out as timestamp when timestamp is invalid, but set next_out to the start of the segment, when a NEWSEGMENT event arrives.
Review of attachment 182178 [details] [review]: ::: ext/ffmpeg/gstffmpegdec.c @@ +2417,2 @@ /* and store the values */ + ffmpegdec->next_out = start; Using the segment start position as the next output timestamp is not correct. Just remove this assignment, then everything should be fine with your other change.
Indeed, it does work, since the code that clips the timestamp to the segment will allow invalid timestamps. But, then, why use next_out at all, in the case of audio? Is it common to get as input timestamp a good timestamp followed by invalid ones?
Yes, for example audio coming from an mpeg transport stream
Created attachment 182835 [details] [review] Fixes the bug
Created attachment 183438 [details] [review] Fixes commit summary.
Review of attachment 183438 [details] [review]: ::: ext/ffmpeg/gstffmpegdec.c @@ +2117,3 @@ /* the next timestamp we'll use when interpolating */ + if (GST_CLOCK_TIME_IS_VALID (in_timestamp)) + ffmpegdec->next_out = out_timestamp + out_duration; Hm, don't you mean out_timestamp here instead of in_timestamp?
Created attachment 183456 [details] [review] Fixed patch
Sorry. This patch was written on top of a gst-ffmpeg version previous to latest git. When fixing the conflict while rebasing it on top of the latest git version, I screwed it up. Fixed now.
commit a3c56f60cdb81a7100352821e2972b3014286ee9 Author: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> Date: Mon Feb 28 21:16:24 2011 +0000 ffdec: Do not use invalid input timestamp as next timestamp. When input buffer timestamps are invalid, next timestamp are used for audio. Then, the next out timestamp is updated with the used timestamp and the calculated duration. However, if the used timestamp is invalid, it should not be used. Otherwise, the next buffer will use a wrong timestamp that is not in the clipped segment, making the buffer to be dropped. This fixes playback with SBTVD MPEG TS streams, using AAC LATM.
Should this be closed, or is there a something pending ?