fix: Add classification streaming support for YOLOView#447
fix: Add classification streaming support for YOLOView#447dmjtian wants to merge 1 commit intoultralytics:mainfrom
Conversation
|
👋 Hello @dmjtian, thank you for submitting a -✅ Define a Purpose: Clearly explain the purpose of your fix or feature in your PR description, and link to any relevant issues. Ensure your commit messages are clear, concise, and adhere to the project's conventions. For more guidance, please refer to our Contributing Guide. Don't hesitate to leave a comment if you have any questions. Thank you for contributing to Ultralytics! 🚀 |
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review
Made with ❤️ by Ultralytics Actions
Good direction aligning classification output with Results.summary(), but there’s a correctness issue in top5 class indices and a likely backward-compatibility break in the legacy boxes payload. Fixing those should make this change safe for existing consumers.
💬 Posted 2 inline comments
| "name" to probs.top1, // Class name | ||
| "class" to probs.top1Index, // Class index (integer) | ||
| "confidence" to probs.top1Conf.toDouble(), | ||
| "top5" to probs.top5.mapIndexed { idx, className -> |
There was a problem hiding this comment.
top5 uses mapIndexed and sets class to the enumeration index, not the model’s class index. That makes the payload incorrect if top-5 class IDs are not 0..4. This will break consumers expecting actual class indices. Use the model-provided top-5 indices instead of idx if available (e.g., probs.top5Indices).
| mapOf( | ||
| "class" to topClass, | ||
| "className" to topClass, | ||
| "name" to probs.top1, // Class name |
There was a problem hiding this comment.
💡 MEDIUM: Backward compatibility is likely broken: the previous payload included className and classIndex keys inside boxes, but the new payload replaces them with name and class. Existing apps consuming the legacy fields will fail. Consider keeping the old keys in addition to the new ones for the compatibility path.
|
Thanks @dmjtian—this looks like the right fix for streaming; can you make sure the emitted classification payload mirrors the core |
Summary
Adds classification result processing to
convertResultToStreamData()method in Android'sYOLOView.kt, enabling real-time classification streaming support.Problem
Currently, classification task works for single image prediction (via
YOLO.predict()) thanks to PR #418, but real-time streaming (viaYOLOViewwithonResultcallback) returns empty results.Current Behavior
Logs:
Classifier succeeds but results array is empty because
convertResultToStreamData()doesn't processresult.probs.Root Cause
The
convertResultToStreamData()method inandroid/src/main/kotlin/com/ultralytics/yolo/YOLOView.kthandles:result.boxes)result.keypointsList)result.obb)result.probs) - MISSINGSolution
Add classification result processing in
convertResultToStreamData()method, similar to how other task types are handled.Code Changes
File:
android/src/main/kotlin/com/ultralytics/yolo/YOLOView.ktLocation: After line ~1668 (after the
includeDetectionsblock ends)Add this code block:
Optional but recommended: Update the log message to include probs count:
Before:
After:
Testing
Test Environment
yolo11n-cls.tfliteYOLOTask.classifyBefore Fix
After Fix
Test Code
Design Decisions
Why add to
detectionsarray?YOLOResult.fromMap()deserializationWhy use full image bounding box?
Why check
config.includeClassifications?YOLOStreamingConfigsettingsImpact
Fixed
YOLOViewwithonResultreturns classification resultsNot Affected
YOLO.predict()) - already works (PR Fix Android CLASSIFY task: ensure FloatArray compatibility (Issue #413 follow-up) #418)Related
Checklist
Notes
This PR focuses on Android implementation. iOS may need a similar fix in
ios/Classes/YOLOView.swiftif the issue exists there.Author: @dmjtian (original contributor of PR #418)