Repository navigation
fix(rocdecode): do not write to a popped frame in ffmpeg_video_dec - #14891
Merged
jeffqjiangNew merged 1 commit intoNov 3, 2026
Merged
Conversation
FFMpegVideoDecoder::ReconfigureDecoder() had two bugs in its frame cleanup loop: - It called vp_frames_.pop_back() and then wrote p_frame->frame_ptr = nullptr. p_frame points to the element that pop_back() just removed, so the write uses a destroyed object. - pop_back() was inside if (p_frame->frame_ptr). A frame with a null frame_ptr was never popped, so while (!vp_frames_.empty()) never ended and the decoder hung while holding mtx_vp_frame_. Pop every frame after the optional free, and drop the write, like the same loop in utils/rocvideodecode/roc_video_dec.cpp. Found by cppcheck 2.18.3 (invalidContainer). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbyZduM5A6iscMiP5UPGfb
randomizedcoder
requested review from
AryanSalmanpour,
jeffqjiangNew,
kiritigowda and
rrawther
as code owners
November 3, 2026 06:53
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the decoder frame-cleanup change.
No open issues 🎉
What changed in this PR
Fixes frame cleanup during FFmpeg decoder reconfiguration in rocdecode, addressing the invalid access reported in #14890.
Changes:
- Removes the write through
p_frameafter its vector element has been removed. - Pops each frame after optional buffer cleanup, ensuring null frame pointers cannot stall the loop.
| File | Description |
|---|---|
projects/rocdecode/utils/ffmpegvideodecode/ffmpeg_video_dec.cpp |
Corrects frame cleanup order in ReconfigureDecoder(). |
Review effort: Balanced
rrawther
approved these changes
Nov 3, 2026
jeffqjiangNew
approved these changes
Nov 3, 2026
systems-assistant Bot
pushed a commit
to ROCm/rocDecode
that referenced
this pull request
Nov 3, 2026
ffmpeg_video_dec (#14891) ## Motivation `FFMpegVideoDecoder::ReconfigureDecoder()` writes through `p_frame` after `vp_frames_.pop_back()` removes the element it points to. cppcheck 2.18.3 reports this as `invalidContainer`. ## Technical Details Remove the write after `pop_back()` and pop each frame after optional buffer cleanup. This also ensures a null `frame_ptr` cannot prevent the loop from progressing. The cleanup order matches `utils/rocvideodecode/roc_video_dec.cpp`. ## Issue Tracking Closes #14890 ## Test Plan Build with FFmpeg/HIP and exercise decoder reconfiguration with buffered frames. ## Test Result Source review and `git diff --check` passed. No FFmpeg/HIP build or runtime test was run locally. No CI results are available on this fork draft; upstream build/runtime validation remains pending. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> [rocm-systems] ROCm/rocm-systems#14891 (commit bfe7697)
Contributor
TheRock Submodule Bump ActivityNewest first
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
FFMpegVideoDecoder::ReconfigureDecoder()writes throughp_frameaftervp_frames_.pop_back()removes the element it points to. cppcheck 2.18.3 reports this asinvalidContainer.Technical Details
Remove the write after
pop_back()and pop each frame after optional buffer cleanup. This also ensures a nullframe_ptrcannot prevent the loop from progressing. The cleanup order matchesutils/rocvideodecode/roc_video_dec.cpp.Issue Tracking
Closes #14890
Test Plan
Build with FFmpeg/HIP and exercise decoder reconfiguration with buffered frames.
Test Result
Source review and
git diff --checkpassed. No FFmpeg/HIP build or runtime test was run locally. No CI results are available on this fork draft; upstream build/runtime validation remains pending.