GNOME Bugzilla – Bug 676630
matroskademux, avidemux: implement accurate keyframe snapping seeks
Last modified: 2018-11-03 14:46:25 UTC
When the ACURATE flag is set for snapping keyframe seeks, we seek to the previous known keyframe and scan forward till the required keyframe is found. For backward snapping, this will require an extra seek. Note that this will be unneeded if all keyframes are indexed. I have no idea whether Matroska requires this or not.
Created attachment 214747 [details] [review] matroskademux: implement accurate keyframe snapping seeks
Created attachment 214781 [details] [review] Fix a buffer leak when scanning.
Patch committed: http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?h=0.10&id=cc242a656348fd0c5d7cbc2fc34de43aa3a33304
Just a note: the patch in comment 2 still has a deadlock issue with before snaps. Please do not push yet. This bug is still open for now.
Comment on attachment 214781 [details] [review] Fix a buffer leak when scanning. This has not been pushed yet, I need to work out how to fix that deadlock before it can be pushed.
Created attachment 215127 [details] [review] matroskademux: implement accurate keyframe snapping seeks When the ACCURATE flag is set for snapping keyframe seeks, we seek to the previous known keyframe and scan forward till the required keyframe is found. For backward snapping, this will require an extra seek.
Deadlock fixed, and various niggles with the backseek painstakingly unniggled.
Created attachment 215168 [details] [review] avidemux: implement accurate keyframe snapping seeks When the ACCURATE flag is set for snapping keyframe seeks, we seek to the previous known keyframe and scan forward till the required keyframe is found. For backward snapping, this will require an extra seek.
Similar patch for avidemux. Seeking now works fine with backward and forward snaps, however there's trouble with going back to PLAYING after a backward seek, I'm pretty sure it's to do with segment (likely the closing segment) but I just can't find the right magic for it, so I'm posting this for now as I'm out of ideas to try for this.
There are some comments here, either on style (avoiding the noise due to !! -> ! ! changes) or on semantics (no closing segment should be sent, because one should prevent any segment sent before knowing when/where to push). But mainly, I seem to be missing how this code will end up at a different keyframe than if there were no ACCURATE flag. If no such flag, the index would be consulted to locate the keyframe ahead or behind the requested position, and then a seek to position follows. If such a flag, there is a seek to the (index defined) keyframe position before the requested position. Then scanning continues, but that scan can never find keyframes that are not in the index, since afaik keyframe-ness is defined by the index. A scan might be "useful" in matroska since the index might only indicate cluster starts, and there can be more keyframes than at cluster start (keyframes so *optionally* marked at their position), but for avi there are only keyframes as indicated in the index (until a parser says otherwise).
In case not clear, the above comment is for the avidemux patch ...
If avi indices are always indicating every keyframe, then you're right that the scanning code will be pointless. I thought avi indices were not mandated though. About the segment handling, they always confuse me :( Won't work without those though, so maybe there's a deeper bug that's waiting for me to find.
Maybe some (pretty rare) AVI do not have an index, but iirc then the whole pull mode code is already in trouble (and more so the seeking part that is already relying on that). Also some style comments on the matroska patch, and I am not sure it is (very) safe in that case either, since matroska only specifies keyframe info in SimpleBlock element, and that one need not be used (only possibly so in v2). So everything might be considered a keyframe, and then the scan might come up with worse result (than not doing one, particularly for _AFTER).
I'm not sure I understand here. Are you saying that the Matroska layer might be giving wrong information about which frames are keyframes (I don't know the format) ?
Yes, sort-of. The Matroska layer might not be giving any information at all whether or not "some blob" is a keyframe (see also the effect it has in bug #639710).
Why is this tagged with '[0.10]' ? What is 0.10-specific about it?
Ping?
AFAIK there is indeed nothing 0.10 specific about it. On the other hand, as indicated above, due to format semantics, I believe the matroskademux one is but marginally functionally relevant, and the avidemux one even less.
What about the patches?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/62.