-
Notifications
You must be signed in to change notification settings - Fork 9.9k
replay: fix video decoding from files with audio streams #35715
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
2c1d33a
to
f4c9639
Compare
69d8153
to
bd85dec
Compare
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.
Pull Request Overview
This PR fixes FrameReader
to correctly handle video files that include audio streams by explicitly finding and using the video stream rather than assuming stream 0.
- Use
av_find_best_stream
to locate the video stream and store its index invideo_stream_idx_
- Filter packet reading and decoder acquisition to the identified video stream
- Revise decoding loop to skip non-video packets and return the requested frame
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tools/replay/framereader.h | Added video_stream_idx_ member to track stream |
tools/replay/framereader.cc | Updated stream selection, packet filtering, and decoding logic |
Comments suppressed due to low confidence (1)
tools/replay/framereader.cc:83
- Consider adding a test with a sample file containing both audio and video streams to verify that loadFromFile correctly identifies the video stream and processes frames without audio interference.
video_stream_idx_ = av_find_best_stream(input_ctx, AVMEDIA_TYPE_VIDEO, -1, -1, NULL, 0);
break; | ||
} | ||
} | ||
|
||
auto pos = reader->packets_info[from_idx].pos; | ||
auto pos = reader->packets_info[current_idx].pos; | ||
int ret = avformat_seek_file(reader->input_ctx, 0, pos, pos, pos, AVSEEK_FLAG_BYTE); |
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.
The call to avformat_seek_file uses stream index 0, which may refer to an audio stream if present. Replace the hardcoded 0 with reader->video_stream_idx_ to seek the correct video stream.
int ret = avformat_seek_file(reader->input_ctx, 0, pos, pos, pos, AVSEEK_FLAG_BYTE); | |
int ret = avformat_seek_file(reader->input_ctx, reader->video_stream_idx_, pos, pos, pos, AVSEEK_FLAG_BYTE); |
Copilot uses AI. Check for mistakes.
rError("Failed to decode frame at index %d", current_idx); | ||
return false; |
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.
decodeFrame may return null when the packet does not immediately yield a frame; instead of erroring out here, you should continue reading subsequent packets until the requested frame is decoded.
rError("Failed to decode frame at index %d", current_idx); | |
return false; | |
rWarning("Frame not decoded, continuing to read subsequent packets..."); | |
continue; |
Copilot uses AI. Check for mistakes.
Fixes
FrameReader
to properly handle video files that contain audio streams.resolve: #35711