List only the candidate frames that reached the disk - #216
Merged
Conversation
When strict diversity leaves fewer candidates than were asked for, a second pass tops the list up and skips anything already chosen with `if c in selected`. A candidate is a dict carrying its own numpy frame, and `in` compares those element-wise, so the check raised "the truth value of an array with more than one element is ambiguous" and took the whole command down with it. Every caller then saw no frames at all rather than the two or three that had already been picked, and the clip got no picture. Identity is what the check meant: the same candidate, not an equal one.
cv2.imwrite answers a failed write by returning False rather than raising, and on a render box a full disk is the usual reason. The frame went into the returned list regardless, so thumbnail-options handed its caller a path to a file that is not there: the cloud worker's upload of it failed on the stat, and thumbnail-render refused it with "no frame at ...", which is the guard added with the last thumbnail fixes reporting a problem made here. Skip a frame that did not get written, and log which one it was.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe thumbnail pipeline now compares candidate frames by identity during relaxed diversity selection. It also checks image write results, logs failures, and skips frames that were not saved. ChangesThumbnail pipeline
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Thumbnail extraction now retains valid candidate selection behavior and avoids returning paths for frames that were not written, preventing downstream rendering from receiving missing files. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merged
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
1 participant
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.
Two faults in
extract_candidate_frames, both of which end with a clip having no picture.A frame that was never written was reported anyway.
cv2.imwriteanswers a failed write by returningFalserather than raising, and on a render box a full disk is the usual reason. The path went into the returned list regardless, so the cloud worker's upload of that frame failed on the stat andthumbnail-renderrefused it withno frame at ...— the guard from #214 reporting a problem made here. Now the frame is skipped and logged.The top-up pass crashed on numpy. When strict diversity leaves fewer candidates than were asked for, a second pass fills the list and skipped what was already chosen with
if c in selected. A candidate is a dict carrying its own numpy frame, soincompared those element-wise and raisedthe truth value of an array with more than one element is ambiguous, taking the command down and leaving the caller with nothing rather than the two or three frames already picked. Identity is what the check meant.Reported from production:
thumbnail render failed: no frame at /var/lib/podcli-work/.../thumbs-.../frames/frame_2.jpg.The matching guard on the cloud side (ignore a listed frame that is not on disk) is on podcli-cloud main as a262be5.
Summary by CodeRabbit