GNOME Bugzilla – Bug 738570
flvdemux: Fix support for seeking flags
Last modified: 2015-08-16 13:39:50 UTC
flvdemux seems unable to seek accurately. When seeking a playbin playing a FLV file with a h.264 stream, seeks always go to key frames even though FLAG_ACCURATE is set. Test files follow.
Created attachment 288572 [details] Archive containing test files test.flv and test.mp4 contain identical h.264 streams with a GOP size of 5 seconds. The FLV file can be sought in in 5-second increments only. The MP4 file can be sought in accurately.
Are you setting FLAG_KEYUNIT? If not it shouldn't go to keyframes even without FLAG_ACCURATE.
No, I don't. gst-play-1.0 --interactive also shows the issue.
just to prevent confusion: without FLAG_KEYUNIT the demuxer is supposed to start with a keyframe too obviously... but the segment will be adjusted so that the first frame within the segment is the seek position, and not the keyframe position. With the FLAG_KEYUNIT the first frame within the segment will be the keyframe, not the seek position
Created attachment 290510 [details] [review] patch 01/02
Created attachment 290511 [details] [review] patch 02/02 Seems to work when using gst-launch-1.0 with navseek. gst-play-1.0 --interactive does KEY_UNIT seeks, which also still work.
Thanks for working on this. The first patch looks mostly fine to me. The second patch looks a bit weird to me. Not wrong necessarily, it's just that we should really never generate a segment event start position just by 'locking on' the first timestamp, we should always generate a seek event based on the seek request, possibly adjusted as per the keyframe position, but not anything else. I suspect what this code (ca. line 1146 in parse_audio_tag and later for video) is supposed to do is to care for a non-0 based timeline. Perhaps this needs to be done differently, or the seek handlers should just generate the new segment event unconditionally in all cases.
I don't have time to delve deeper into this matter right now. Can the fixes we already have be applied?
I'd rather not increase the WTF-factor of the code.
Created attachment 299509 [details] [review] patch Well then, one attempt at reducing the WTF factor. Used test commands: gst-launch-1.0 playbin uri=file://$PWD/test.flv video-sink="navseek ! autovideosink" (does normal seeks) gst-play-1.0 --interactive test.flv (does KEY_UNIT seeks)
Tim hasn't found any time to review the patch. Is there anyone else who can?
Review of attachment 299509 [details] [review]: Looks good in general. I'm concern about the segment because start == time. But at the same time, this won't make anything worst then it was before. I would suggest merging. Setting as reviewed, so see if there is objection (or I missed something obvious). ::: gst/flv/gstflvdemux.c @@ +2563,3 @@ } + if (demux->segment.flags & GST_SEGMENT_FLAG_SEGMENT) { Hmm, indeed. @@ +2647,2 @@ /* Adjust the segment so that the keyframe fits in */ + segment->start = segment->time = time; I can't find if we properly offset the start/stop later to compensate the DTS shift. In that case, time will be a little behind start (stream time will start from zero, while TS won't match streamtime).
IIRC this is the last piece of the puzzle needed to accurately timeshift RTMP streams.
Thanks, this patch also falls in the "can't be worst" considering the code was not event checking the flags from the right values. My reading of it seems to indicate it's right, or at least clearly the right direction. commit 439f98ed9a31c836da97176b58663a4a8fe64f0b Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv> Date: Wed Nov 12 12:08:58 2014 +0100 flvdemux: Handle seek flags properly Allows for non-keyframe seeks. https://bugzilla.gnome.org/show_bug.cgi?id=738570