Skip to content

fix: The CVTE customer requests to block irrelevant cameras.#452

Closed
lichaofan2008 wants to merge 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle
Closed

fix: The CVTE customer requests to block irrelevant cameras.#452
lichaofan2008 wants to merge 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle

Conversation

@lichaofan2008
Copy link
Contributor

The CVTE customer requests to block irrelevant cameras and retain only the camera named "Smart Camera : Smart Camera" for camera use.
CVTE客户要求屏蔽无关摄像头,只保留名为Smart Camera : Smart Camera的摄像头供相机使用。

Bug: https://pms.uniontech.com/bug-view-349401.html

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @lichaofan2008, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

The CVTE customer requests to block irrelevant cameras and retain only the camera named "Smart Camera : Smart Camera" for camera use.
CVTE客户要求屏蔽无关摄像头,只保留名为Smart Camera : Smart Camera的摄像头供相机使用。

Bug: https://pms.uniontech.com/bug-view-349401.html
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要涉及三个文件:v4l2_core.cv4l2_devices.cvideowidget.cpp。修改的核心目的是增加对特定厂商摄像头的过滤逻辑,以及改进设备初始化时的错误处理。

以下是对这段代码的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

v4l2_core.c:

  • 改进点:原代码在 v4l2core_get_device_index 返回负值时,强制将索引设为 0。这掩盖了设备未找到的错误,可能导致后续逻辑操作错误的设备(例如尝试打开 /dev/video0 而非用户指定的设备)。
  • 当前修改:当索引小于 0 时,打印错误信息,清理资源并返回 NULL
  • 评价:逻辑正确,符合防御性编程原则。调用方现在可以正确处理初始化失败的情况。

v4l2_devices.c:

  • 改进点:将获取 USB 父设备的逻辑提前,以便在枚举设备列表时尽早根据 VID/PID 进行过滤。
  • 逻辑判断
    • restricted_vendors 数组定义了需要限制的厂商。
    • 逻辑是:如果设备是受限厂商(SeeVision 或 CVTE),并且设备名称以 "Smart Camera" 开头,但不等于 "Smart Camera : Smart Camera",则忽略该设备。
  • 潜在逻辑风险:注释中提到 "seevision厂家...要求限制某些设备的可见性",但代码中的逻辑是 if (is_restricted && ... != 0)。这意味着对于受限厂商,只有特定名称("Smart Camera : Smart Camera")的设备会被保留,其他以 "Smart Camera" 开头的设备会被过滤。请务必确认这是预期的业务逻辑(即黑名单模式还是白名单模式)。如果意图是过滤掉所有 "Smart Camera" 开头的设备,逻辑可能需要微调。

videowidget.cpp:

  • 改进点:在启动摄像头前,检查配置文件中的设备是否有效。
  • 逻辑判断:调用 v4l2core_get_device_index 检查设备是否存在。如果不存在,则使用空字符串调用 switchCamera
  • 评价:逻辑清晰,防止了尝试打开无效设备。

2. 代码质量

  • 资源释放
    • v4l2_devices.c 的过滤逻辑中,当决定 continue 跳过当前设备时,正确调用了 getUdev()->m_udev_device_unref(dev);。这非常重要,因为 udev_device_get_parent_with_subsystem_devtype 返回的引用计数增加了,必须手动释放,否则会导致内存泄漏。
  • 代码风格
    • restricted_vendors 数组使用了 static const,且定义清晰,易于维护。
    • 宏定义/常量定义(如 SMART_CAMERA_NAME_PREFIX)集中管理,比硬编码字符串要好。
  • 错误信息
    • fprintf(stderr, ...)qWarning() 的使用得当,有助于调试。

3. 代码性能

  • 字符串比较
    • v4l2_devices.c 中使用了 strncmp 进行前缀匹配。由于是在枚举设备过程中执行,且设备数量通常很少(个位数或十位数),这里的性能开销可以忽略不计。
  • 函数调用开销
    • v4l2core_get_device_indexvideowidget.cpp 中被调用。该函数通常涉及遍历设备列表。由于只在初始化时调用一次,性能影响不大。

4. 代码安全

  • 空指针检查
    • v4l2_devices.c 中,获取 vidpid 后,在 strcmp 前进行了 vid && pid 的检查,防止空指针解引用,这很好。
    • v4l2_core.c 中,vd->this_device 检查后进行了 clean_v4l2_dev,防止资源泄漏。
  • 缓冲区安全
    • 使用了 strncmp 而非 strcmp,并且使用了 strlen 计算长度,避免了潜在的缓冲区溢出风险(前提是 v4l2_cap.card 是以 null 结尾的字符串,通常 V4L2 结构体保证这一点)。

综合改进建议

  1. v4l2_devices.c 逻辑确认
    建议再次确认过滤逻辑:

    if (is_restricted && strncmp(..., PREFIX) == 0) {
        if (strncmp(..., FULL_NAME) != 0) {
            // 忽略
        }
    }

    这意味着:如果是受限厂商,且名字是 "Smart Camera..." 但不是 "Smart Camera : Smart Camera",则忽略。
    如果业务需求是"如果是受限厂商,且名字是 'Smart Camera : Smart Camera',则忽略",或者"如果是受限厂商,全部忽略",那么这里的逻辑需要反转或修改。目前的逻辑看起来像是一个白名单过滤(只允许特定名字的设备通过)。

  2. v4l2_core.c 错误处理
    修改后的代码返回 NULL。请确保 v4l2core_init_dev 的所有调用者都检查了返回值,否则调用者可能会对 NULL 指针进行操作,导致崩溃。虽然这是调用者的责任,但在代码审查中值得注意。

  3. videowidget.cpp 的用户体验
    当检测到无效设备时,代码回退到 switchCamera("", "")。建议确认 switchCamera 内部对空字符串的处理是否符合预期(例如弹出一个选择设备的对话框,或者保持无设备状态),而不是静默失败。

  4. v4l2_devices.c 变量作用域
    restricted_vendors 数组被定义为 static const。如果这段代码在多线程环境下运行(虽然 enum_v4l2_devices 通常是单线程调用的),只读数据是安全的。

总结
这段代码的修改在逻辑上是严谨的,增加了必要的错误处理和业务逻辑过滤。资源管理(udev unref)做得很好。主要需要关注的是过滤逻辑是否符合实际业务需求,以及对 NULL 返回值的级联检查。

Copy link

@max-lvs max-lvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1

static const char *SMART_CAMERA_NAME = "Smart Camera : Smart Camera";

const char *vid = getUdev()->m_udev_device_get_sysattr_value(dev, "idVendor");
const char *pid = getUdev()->m_udev_device_get_sysattr_value(dev, "idProduct");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不应该在 libcam里面搞特殊定制,特殊定制应该在 camera 业务代码里面实施,通过osconfig 来做

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants