feat: Add the logic for switching cameras by USB groups.#461
Conversation
Reviewer's GuideImplements USB camera group–aware switching logic in videowidget, controlled via a new DConfig-driven flag stored in DataManager, and introduces helper logic to compute groups based on device location while preserving the old behavior when grouping is disabled. Sequence diagram for USB group–aware camera switchingsequenceDiagram
actor User
participant VideoWidget as videowidget
participant DataManager
participant V4L2 as V4L2Devices
User ->> VideoWidget: onChangeDev()
VideoWidget ->> V4L2: get_device_list()
V4L2 -->> VideoWidget: v4l2_device_list_t devlist
VideoWidget ->> DataManager: isEnableUsbGroup()
DataManager -->> VideoWidget: bool enableUsbGroup
alt USB grouping disabled or only one group
Note over VideoWidget,V4L2: Legacy device-based switching
alt exactly 2 devices
VideoWidget ->> VideoWidget: iterate devlist
VideoWidget ->> VideoWidget: switchCamera(other_device)
else 0 or more than 2 devices
alt no devices
VideoWidget ->> DataManager: setdevStatus(NOCAM)
VideoWidget ->> VideoWidget: showNocam()
else devices available
VideoWidget ->> VideoWidget: find current device index
alt current is last device
VideoWidget ->> VideoWidget: switchCamera(first_device)
else current not last
VideoWidget ->> VideoWidget: switchCamera(next_device)
end
alt current device unknown
VideoWidget ->> VideoWidget: switchCamera(first_device)
end
end
end
else USB grouping enabled with multiple groups
VideoWidget ->> VideoWidget: getUSBCameraGroup(devlist, vGroupData)
VideoWidget ->> VideoWidget: groupNum = vGroupData.count()
alt exactly 2 groups
Note over VideoWidget,V4L2: Toggle between two groups
VideoWidget ->> VideoWidget: iterate groups
VideoWidget ->> VideoWidget: switchCamera(group[i].first_device)
else more than 2 groups
alt no devices
VideoWidget ->> DataManager: setdevStatus(NOCAM)
VideoWidget ->> VideoWidget: showNocam()
else devices available
VideoWidget ->> VideoWidget: find current group index
alt current is last group
VideoWidget ->> VideoWidget: switchCamera(first_group.first_device)
else current not last
VideoWidget ->> VideoWidget: switchCamera(next_group.first_device)
end
alt current device unknown
VideoWidget ->> VideoWidget: switchCamera(first_group.first_device)
end
end
end
end
Class diagram for updated DataManager and videowidget USB group logicclassDiagram
class DataManager {
- static DataManager *m_dataManager
- bool m_H264EncoderExists
- bool m_isSupportCameraSwitch
- bool m_isPreviewNoDelay
- bool m_enableUsbGroup
+ static DataManager *instance()
+ void setPreviewNoDelay(bool enable)
+ bool isPreviewNoDelay()
+ void setEnableUsbGroup(bool enable)
+ bool isEnableUsbGroup() const
+ void setdevStatus(int status)
}
class v4l2_dev_sys_data_t {
+ const char *device
+ const char *name
+ const char *location
}
class v4l2_device_list_t {
+ int num_devices
+ v4l2_dev_sys_data_t *list_devices
}
class videowidget {
+ void onChangeDev()
- int switchCamera(const char *device, const char *devName)
- int getUSBCameraGroup(v4l2_device_list_t *devlist, QVector_QPair_QString_QVector_v4l2_dev_sys_data_t_ptr & vGroupData)
}
DataManager <.. videowidget : uses
v4l2_device_list_t <.. videowidget : uses
v4l2_dev_sys_data_t <.. v4l2_device_list_t : contains
class QVector_QPair_QString_QVector_v4l2_dev_sys_data_t_ptr {
}
QVector_QPair_QString_QVector_v4l2_dev_sys_data_t_ptr <.. videowidget : groups
Flow diagram for DConfig-driven USB group enable flagflowchart LR
A[DConfig enableUsbGroup key] --> B[main reads enableUsbGroup]
B --> C{enableUsbGroup value}
C -->|true or false| D[DataManager::setEnableUsbGroup]
D --> E[videowidget::onChangeDev]
E --> F{DataManager::isEnableUsbGroup}
F -->|false or groupNum == 1| G[Legacy device-based switching]
F -->|true and groupNum > 1| H[USB group-based switching via getUSBCameraGroup]
H --> I[switchCamera using group representatives]
G --> I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
getUSBCameraGroupreturn value is documented as0 成功 其他 失败, but it actually returns the number of groups (vGroupData.count()); consider updating the comment or changing the return semantics to avoid confusion. - The camera-switching logic for grouped and ungrouped devices has a lot of duplicated patterns (e.g. handling no devices and cycling to the next item); extracting a shared helper or normalizing on an index-based iteration would make this easier to maintain.
- When
groupNum == 2you use a different branch than for othergroupNum > 1values even though the selection logic is very similar; it may be clearer to unify the code path for allgroupNum >= 2and just rely onvGroupData.count().
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `getUSBCameraGroup` return value is documented as `0 成功 其他 失败`, but it actually returns the number of groups (`vGroupData.count()`); consider updating the comment or changing the return semantics to avoid confusion.
- The camera-switching logic for grouped and ungrouped devices has a lot of duplicated patterns (e.g. handling no devices and cycling to the next item); extracting a shared helper or normalizing on an index-based iteration would make this easier to maintain.
- When `groupNum == 2` you use a different branch than for other `groupNum > 1` values even though the selection logic is very similar; it may be clearer to unify the code path for all `groupNum >= 2` and just rely on `vGroupData.count()`.
## Individual Comments
### Comment 1
<location path="src/src/videowidget.h" line_range="429" />
<code_context>
+ * @param vGroupData 分组
+ * @return 0 成功 其他 失败
+ */
+ int getUSBCameraGroup(v4l2_device_list_t *devlist, QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> &vGroupData);
+
/**
</code_context>
<issue_to_address>
**issue (bug_risk):** The documented return semantics for getUSBCameraGroup disagree with the implementation.
The comment documents a 0/非0 success/failure contract, but the function actually returns `vGroupData.count()`, where 0 means “no groups” and a positive value is the number of groups. Please either align the implementation with the documented contract or update the comment to state that the return value is the number of USB groups found (0 if none).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
90e0712 to
3bd6ba0
Compare
Add the logic for switching cameras by USB groups. 增加按USB分组切换摄像头的逻辑。 代码来自xiwo分支。
3bd6ba0 to
90998c7
Compare
deepin pr auto review代码审查报告1. 语法与逻辑
2. 代码质量
3. 性能优化
4. 安全性
5. 其他建议
改进后的代码示例int videowidget::groupUSBCameras(v4l2_device_list_t *devlist, QMap<QString, QVector<v4l2_dev_sys_data_t *>> &cameraGroups)
{
if (devlist == nullptr || devlist->num_devices <= 0) {
qWarning() << __func__ << "Invalid device list!";
return 0;
}
for (int i = 0; i < devlist->num_devices; i++) {
QString location = QString::fromUtf8(devlist->list_devices[i].location);
cameraGroups[location].append(&devlist->list_devices[i]);
}
return cameraGroups.size();
}总结本次代码修改主要实现了USB摄像头分组功能,整体逻辑正确,但在性能优化、代码质量和安全性方面仍有改进空间。建议优先解决空指针检查和字符串比较问题,后续可考虑重构分组逻辑以提升可维护性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, max-lvs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
6934da1
into
linuxdeepin:release/eagle
Add the logic for switching cameras by USB groups.
增加按USB分组切换摄像头的逻辑。
代码来自xiwo分支。
Summary by Sourcery
Add configurable USB camera grouping and update camera switching logic to respect these groups while preserving existing behavior when grouping is disabled.
New Features:
Enhancements: