-
Notifications
You must be signed in to change notification settings - Fork 107
Consider hidden frames in FrameFromPTS/Pos when seeking #457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
01cae89 added the AllowHidden parameter to FrameFromPTS/Pos to be able to recognize the current packet in DecodePacket, but kept it set to false in the other invocations to change as little existing behavior as possible. However, it looks like it's actually correct to also allow hidden frames in the other invocations in GetFrame, since those call FrameFromPTS to find out what packet the demuxer ended up at after a seek. FFMS#456 shows a file where this makes a difference, since its first few frames are marked as hidden. Setting AllowHidden to true everywhere in the decoding loop (just not in GetFrameByTime) also causes no regressions on the Doom9 seeking sample collection (https://forum.doom9.org/showthread.php?p=1992736#post1992736).
FFMS2 requires ffmpeg 7.1 now, which fixes this bug.
|
Welp, this breaks the CI, I'll look into it. |
|
Looks like |
|
Funnily enough, only the combination of these two commits breaks the test, each one individually is "fine". But that's just a coincidence. Here's my understanding of what's happening with this file: We've got an mp4 file with an edit list (fun) skipping a small part of the video. When demuxing, ffmpeg outputs the following packets (listed in decoding order): The first edit list entry goes up to PTS 8704, but we get a bunch of additional packets after that marked with When trying to access a frame from the second section (say frame 59 in 0-indexed decoding order) we seek to the second-to-last keyframe, frame 37 in decoding order. Lavf's seek does correctly put us there and the first packet we decode is the keyframe with PTS 9216 (the second one of the two; frame 37 in decoding order). But when trying to figure out what frame we ended up at based on the PTS, we call So I understand now why you'd want to exclude hidden frames in the Since real goal of the FrameFromPTS/Pos calls (both in GetFrame and DecodePacket) are to try and find the position of a given packet in our index, I feel like the proper solution to this is to make this detection more robust, e.g. using a I'll try writing something like this, but I thought I'd write out my thoughts here first. You probably know a lot more than me about all the various edge cases involved here. For example, is it possible for a packet's FilePos to differ between reads, depending what kinds of seeks came before? And similar questions for the PTS, flags, etc. |
See #456 for samples and discussion, as well as the commit message.
Also, while I'm at it, remove the workaround for the h264 decoder bug again, since FFMS2 requires FFmpeg 7.1 now, which includes the fix.