Add non-blocking try_get_next_frame to Capturer
#177
Conversation
cijiugechu
changed the title
Capturertry_get_next_frame to Capturer
WalkthroughAdds a non-blocking frame retrieval method Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant C as Capturer
participant RX as Channel (rx)
participant E as Engine
Caller->>C: try_get_next_frame()
C->>RX: try_recv()
alt Item available
RX-->>C: Item
C->>E: process_channel_item(Item)
alt Frame produced
E-->>C: Frame
C-->>Caller: Ok(Some(Frame))
else No frame yielded
E-->>C: None
C-->>Caller: Ok(None)
end
else Empty
RX-->>C: Empty
C-->>Caller: Ok(None)
else Disconnected
RX-->>C: RecvError
C-->>Caller: Err(RecvError)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/capturer/mod.rs (1)
143-157: Previous concern resolved: implementation now correctly mirrorsget_next_framesemantics.The loop-based implementation addresses the semantic inconsistency flagged in the previous review. By draining all available channel items until a usable frame is found or the channel is empty,
try_get_next_framenow behaves consistently withget_next_frame—the only difference being blocking vs. non-blocking behavior.
Ok(None)unambiguously means "no frame currently available," not "item was filtered."Optional: Consider expanding the doc comment.
While the implementation is clear, users might benefit from more detail:
- /// Attempts to return the next captured frame without blocking. + /// Attempts to return the next captured frame without blocking. + /// + /// Processes all currently available channel items until a usable frame is found. + /// Returns `Ok(None)` if no frames are available at this moment (not that items were filtered). + /// Returns `Err(mpsc::RecvError)` if the capture channel has been disconnected. pub fn try_get_next_frame(&self) -> Result<Option<Frame>, mpsc::RecvError> {
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/capturer/mod.rs(1 hunks)
🔇 Additional comments (1)
src/capturer/mod.rs (1)
148-161: LGTM! Non-blocking semantics correctly implemented.The implementation correctly addresses the previous concern by looping through all available channel items until a usable frame is found. The behavior now matches
get_next_framesemantics while providing non-blocking operation:
- Drains all currently available items in the channel
- Returns the first usable frame via
Ok(Some(frame))- Returns
Ok(None)unambiguously when the channel is empty (not because items were filtered)- Correctly maps
TryRecvError::DisconnectedtoRecvErrorfor consistency with the blocking API
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/capturer/mod.rs (1)
143-161: Implementation looks good; consider clarifying documentation.The implementation correctly mirrors
get_next_frame's semantics by looping through filtered items transparently. The previous semantic inconsistency has been resolved.However, the documentation at line 146 is slightly ambiguous. The phrase "(note that items were filtered)" could be misread as suggesting that
Ok(None)is related to filtered items, when in factOk(None)is returned only when the channel is empty—filtered items are handled transparently in the loop and never cause an early return.Consider this clearer phrasing:
- /// Returns `Ok(None)` if no frames are available at this moment (note that items were filtered). + /// Returns `Ok(None)` if the channel is empty; filtered items are processed transparently.Or alternatively:
- /// Returns `Ok(None)` if no frames are available at this moment (note that items were filtered). + /// Returns `Ok(None)` only when the channel is empty, not because items were filtered.
1 participant
This PR introduces a new non-blocking method
try_get_next_frameto theCapturer.The existing
get_next_frameblocks the calling thread until a frame is available, which can be unsuitable for applications that need to remain responsive, such as GUI applications or game loops.Summary by CodeRabbit