-
Notifications
You must be signed in to change notification settings - Fork 111
fix(audio): guard the decode loop against pathological stalls #430
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
Open
superkc2026
wants to merge
4
commits into
getopenscreen:main
Choose a base branch
from
superkc2026:perf/decode-budget-guard
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+54
−0
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e2e89c9
fix(audio): guard the decode loop against pathological stalls
cb3aed5
fix(audio): reset the demuxer when av_seek_frame fails (CodeRabbit #430)
6d03057
fix(audio): size the decode budget from the actual decode start (Code…
19d4aee
Merge branch 'main' into perf/decode-budget-guard
EtienneLescot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: getopenscreen/openscreen
Length of output: 10367
🏁 Script executed:
Repository: getopenscreen/openscreen
Length of output: 50381
🌐 Web query:
FFmpeg av_seek_frame AVSEEK_FLAG_BACKWARD keyframe actual position documentation audio stream💡 Result:
In FFmpeg, the av_seek_frame function uses the AVSEEK_FLAG_BACKWARD flag to indicate that when an exact match for the requested timestamp is not available, the seek operation should land at a position at or before the target timestamp [1]. Regarding your specific areas of inquiry: Keyframe Behavior and Guarantees The flag AVSEEK_FLAG_BACKWARD does not inherently guarantee that the resulting position will be a keyframe [2][3]. The success of landing on a keyframe depends heavily on the demuxer implementation, the file container, and the underlying stream type [2][4]. While the default behavior (without AVSEEK_FLAG_ANY) is intended to prioritize keyframes, many demuxers—particularly those handling complex containers like MPEG-TS or specific video formats—may return a position that is not a keyframe if an exact keyframe match is not found or if the index information is insufficient [2][3][4]. Users often encounter situations where the first packet returned after a seek has flags = 0, indicating a non-keyframe [3]. Audio Streams FFmpeg's av_seek_frame is a legacy API that operates primarily based on a single stream index [1]. When you provide a stream index, it attempts to seek relative to that stream's timebase [5]. If you pass -1 as the stream_index, the function attempts to guess a default stream, which can lead to unpredictable results if the container has multiple streams (e.g., audio and video) with different interleaving or keyframe characteristics [1][3]. Because audio streams often have different (or effectively non-existent) keyframe structures compared to video, seeking in audio may result in landing at the requested timestamp without the "keyframe" restriction being meaningfully applied, or it may disrupt synchronization if the seek was intended to align video keyframes [1]. Practical Recommendations 1. Expected Behavior: You cannot assume the result of av_seek_frame with AVSEEK_FLAG_BACKWARD will be a keyframe [3][4]. If your application requires a keyframe (e.g., for accurate decoding), you must often manually discard non-keyframe packets or decode forward from a preceding keyframe until you reach the target timestamp [4]. 2. Alternative API: The newer avformat_seek_file function is generally preferred over av_seek_frame as it offers more granular control, including explicitly defining minimum and maximum timestamp bounds, which can lead to more predictable seeking behavior [6][1]. 3. Verification: Always inspect the packet flags of the first packet received after a seek if your application's logic depends on starting from a keyframe [3].
Citations:
Include seek pre-roll in the decode budget.
AVSEEK_FLAG_BACKWARDdoes not guarantee an exact or keyframe-aligned landing point. A successful seek can require substantial decoding beforesource_start_sec, butloop_budget_secsexcludes that pre-roll. Use the actual landing position or a conservative pre-roll bound to prevent valid streams from exceeding the budget.🤖 Prompt for AI Agents