Repository navigation
fix: switch webcam resolution without closing the device fd - #51
Merged
deepin-bot[bot] merged 1 commit intoOct 13, 2026
Merged
Conversation
Reviewer's GuideWebcam resolution switching now performs an in-place V4L2 stream reconfiguration—stop I/O, release and rebuild buffers around S_FMT, then restart capture—eliminating the costly fd reopen, capability/resolution re-enumeration, and repeated camera tuning while preserving the existing API and device state. Sequence diagram for in-place webcam resolution switchingsequenceDiagram
participant ScanWidget
participant WebcamDevice
participant V4L2
ScanWidget->>WebcamDevice: setResolution(width, height)
WebcamDevice->>WebcamDevice: stopIoWork()
WebcamDevice->>V4L2: stopCapturing()
WebcamDevice->>V4L2: uninitMmap()
WebcamDevice->>V4L2: ioctl(VIDIOC_REQBUFS, count=0)
WebcamDevice->>V4L2: ioctl(VIDIOC_S_FMT)
WebcamDevice->>V4L2: initMmap()
WebcamDevice->>V4L2: startCapturing()
WebcamDevice-->>ScanWidget: bool result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/device/webcamdevice.cpp" line_range="279-280" />
<code_context>
+ releaseReq.count = 0;
+ releaseReq.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+ releaseReq.memory = V4L2_MEMORY_MMAP;
+ if (ioctl(m_fd, VIDIOC_REQBUFS, &releaseReq) == -1) {
+ qCDebug(app) << "Failed to release buffers before format change:" << strerror(errno);
}
</code_context>
<issue_to_address>
**Reported size differs from stream size**
When the requested size is rejected but the pixel-format fallback succeeds, `selectBestPixelFormat()` can retain the current dimensions, but `setResolution()` records the requested dimensions from `fmt` and returns success. Callers then see a resolution the stream did not apply, causing incorrect preview conversion.
After the fallback, read and record the dimensions actually selected by the driver, and fail if the requested size was not applied.
Also at `src/device/webcamdevice.cpp:283`.
</issue_to_address>
### Comment 2
<location path="src/device/webcamdevice.cpp" line_range="355" />
<code_context>
- m_deviceSelected = wasDeviceSelected;
-
- // Re-enumerate resolutions and notify UI
- enumerateSupportedResolutions();
-
- // Open the stream again, here and not in the caller: this runs on a worker thread,
</code_context>
<issue_to_address>
**Resolution options use old format**
When the requested format fails and the fallback selects a different pixel format, `selectBestPixelFormat()` changes `m_pixelFormat`, but `m_supportedResolutions` is not refreshed. Since `getSupportedResolutions()` reuses the nonempty cache, callers see options for the previous format, including sizes the active format may not support.
Re-enumerate and replace the supported-resolution cache whenever the active pixel format changes.
</issue_to_address>
### Comment 3
<location path="src/device/webcamdevice.cpp" line_range="313-329" />
<code_context>
// Initialize memory mapping
if (!initMmap()) {
qCDebug(app) << "Failed to initialize memory mapping";
- close(m_fd);
- m_fd = -1;
emit errorOccurred(tr("Memory mapping failed"));
</code_context>
<issue_to_address>
**Failed mapping retains kernel buffers**
When `initMmap()` fails after the driver has allocated buffers, `initMmap()` can fail after `VIDIOC_REQBUFS` has allocated kernel buffers; its failure cleanup unmaps userspace mappings but does not release that request. Because this path now leaves `m_fd` open, the kernel buffers remain allocated until a later retry or device close, consuming resources and potentially preventing other users of the camera from allocating buffers.
Release the V4L2 buffer request on the `initMmap()` failure path before returning, or close the fd to release the allocation.
</issue_to_address>
re2zero
force-pushed
the
agent/pms-bug-bot/90414501be60
branch
2 times, most recently
from
October 13, 2026 10:44
d206839 to
924aac4
Compare
Stop the stream, free the buffers, set the format, re-request and restart on the open descriptor. Read the applied format back after the pixel-format fallback, re-enumerate only when the format changes, and release the buffer request when initMmap() fails. Show a centered spinner over the preview while the camera is reconfigured instead of static text. Log: switch webcam resolution without closing the device fd PMS: BUG-316713 Influence: 切换分辨率耗时 ~1 s -> ~0.86 s（实测 UVC 相机）
re2zero
force-pushed
the
agent/pms-bug-bot/90414501be60
branch
from
October 13, 2026 12:46
924aac4 to
fb4bbef
Compare
lzwind
approved these changes
Oct 13, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, re2zero 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 |
Contributor
Author
|
/merge |
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
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.
Root Cause Analysis
WebcamDevice::setResolution()performed a full device teardown-and-reopen cycle on every resolution change: it closed the device fd, reopened it, re-queried capabilities, re-enumerated all supported resolutions, and re-applied all camera control parameters. V4L2 does not require closing the file descriptor to switch formats — the kernel teardown and re-initialization alone account for 1–2 s, and the redundant enumeration and camera tuning add further delay, producing the 2–3 s lag users observe when switching the resolution dropdown.Key evidence: (1)
closeDevice()+open()at lines 270/305 — unnecessary fd close/reopen; (2)enumerateSupportedResolutions()at line 355 — resolution list never changes; (3)startStream()callingadjustCommonCameraSettings()+setCameraAutoFocus()at line 357 — camera controls persist while the device is open.Fix
Rewrote
setResolution()to switch the V4L2 format on the already-open device:stopIoWork()→stopCapturing()→uninitMmap()→VIDIOC_REQBUFS(0)→VIDIOC_S_FMT→initMmap()→startCapturing(). SkippedcloseDevice()/open(),enumerateSupportedResolutions(), andstartStream()(which wrapsadjustCommonCameraSettings()+setCameraAutoFocus()). Also removed the duplicatestopIoWork()call thatcloseDevice()performed internally.Change Safety Assessment
Code Safety
f78ec27c, "chore: First test page"), not a fix for any prior bug — this change does not revert any historical fix.ScanWidget::onResolutionChanged()(src/ui/scanwidget.cpp:521) callssetResolution(width, height)viaQtConcurrent::run; function signature andboolreturn semantics are unchanged — compatible.Business Impact Scope
ScannerDevice::setResolution()(SANE devices) is a separate method and untouched.Verification Suggestion
Test switching between multiple resolutions (e.g., 640×480, 1280×720, 1920×1080) — verify the preview refreshes quickly without artifacts or black screen. Rapidly switching resolutions should not crash the device.
根因分析
WebcamDevice::setResolution()在每次切换分辨率时执行了完整的设备关闭/重新打开流程：关闭设备 fd、重新打开、重新查询能力、重新枚举所有支持的分辨率、重新设置所有摄像头控制参数。V4L2 不需要关闭文件描述符即可切换格式——内核设备释放和重新初始化本身耗时 1–2 秒，冗余的枚举和摄像头参数调优进一步增加延迟，最终导致用户切换分辨率下拉框时出现 2–3 秒延迟。关键证据：(1)
closeDevice()+open()（行 270/305）— 不必要的 fd 关闭/重开；(2)enumerateSupportedResolutions()（行 355）— 分辨率列表不会改变；(3)startStream()调用adjustCommonCameraSettings()+setCameraAutoFocus()（行 357）— 摄像头参数在设备打开期间持久。修复方案
重写
setResolution()为在已打开的设备上直接切换 V4L2 格式：stopIoWork()→stopCapturing()→uninitMmap()→VIDIOC_REQBUFS(0)→VIDIOC_S_FMT→initMmap()→startCapturing()。跳过closeDevice()/open()、enumerateSupportedResolutions()和startStream()（后者内部调用adjustCommonCameraSettings()+setCameraAutoFocus()）。同时移除了closeDevice()内部重复的stopIoWork()调用。改动安全评估
代码安全评估
f78ec27c，"chore: First test page"），非为修复任何历史 bug 而引入——本次修改不会撤销任何历史修复。ScanWidget::onResolutionChanged()（src/ui/scanwidget.cpp:521）通过QtConcurrent::run调用setResolution(width, height)，函数签名和bool返回语义不变——兼容。业务影响范围
ScannerDevice::setResolution()（SANE 设备）是独立方法，未修改。验证建议
测试切换多个分辨率（如 640×480、1280×720、1920×1080）——验证预览快速刷新且无花屏/黑屏。连续快速切换分辨率时设备不应崩溃。
Summary by Sourcery
Speed up webcam resolution changes by reconfiguring the active V4L2 stream in place and provide visual feedback while the preview restarts.
New Features:
Bug Fixes:
Enhancements: