fick from v20#504
Merged
Merged
Conversation
Use the DConfig mechanism to enable the configuration for the preferred resolution. 使用DConfig机制，开放首选分辨率的配置功能。 pick form: 2cdcb53 Task: https://pms.uniontech.com/task-view-387315.html
Reviewer's GuideIntroduce a configurable preferred resolution flow from config -> DataManager -> v4l2 core, replace the previous "largest valid resolution" selection logic, and add a DConfig-based low-performance-device override to the existing board-based detection. Sequence diagram for preferred resolution config to v4l2 core selectionsequenceDiagram
actor User
participant Main as main
participant DConfig as DConfig
participant DataManager as DataManager
participant V4L2Formats as v4l2_formats
participant V4L2Core as v4l2core_prepare_valid_resolution
User->>Main: start application
Main->>DConfig: DConfig::create("org.deepin.camera", "org.deepin.camera.encode")
Main->>DConfig: value("preferredResolution")
DConfig-->>Main: QString preferredResolution
Main->>DataManager: setPreferredResolution(preferredResolution)
Main->>DataManager: getPreferredResolution()
DataManager-->>Main: QSize size
Main->>V4L2Formats: set_preferred_resolution(size.width(), size.height())
User->>V4L2Core: v4l2core_prepare_valid_resolution(vd)
V4L2Core->>V4L2Formats: get_preferred_resolution_width()
V4L2Formats-->>V4L2Core: pref_w
V4L2Core->>V4L2Formats: get_preferred_resolution_height()
V4L2Formats-->>V4L2Core: pref_h
V4L2Core->>V4L2Core: scan resolutions, find preferred or max
V4L2Core-->>User: selected resolution my_width x my_height
Sequence diagram for low performance device detection with DConfig overridesequenceDiagram
participant GlobalUtils as GlobalUtils
participant DConfig as DConfig
participant SystemInfo as com.deepin.system.SystemInfo
GlobalUtils->>GlobalUtils: loadCameraConf()
GlobalUtils->>DConfig: DConfig::create("org.deepin.camera", "org.deepin.camera.encode")
DConfig-->>GlobalUtils: DConfig instance
GlobalUtils->>DConfig: keyList()
GlobalUtils->>DConfig: value("isLowPerformanceDevice")
DConfig-->>GlobalUtils: QVariant bool
GlobalUtils->>GlobalUtils: m_IsLowPerformanceDevice = toBool()
GlobalUtils->>GlobalUtils: isLowPerformanceBoard()
alt m_IsLowPerformanceDevice == true
GlobalUtils-->>GlobalUtils: return true
else m_IsLowPerformanceDevice == false
GlobalUtils->>SystemInfo: query DMI via DBus
SystemInfo-->>GlobalUtils: DMIInfo
GlobalUtils->>GlobalUtils: check m_LowPerformanceBoards
GlobalUtils-->>GlobalUtils: return result
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
lzwind
previously approved these changes
Jul 21, 2026
![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 1 issue, and left some high level feedback:
- The new resolution selection path in v4l2core_prepare_valid_resolution now ignores the previous my_width/my_height-based logic and always chooses either the preferred or the max resolution; consider preserving the old behavior (e.g. respecting an initial target resolution and the 3840x2160 limit) or explicitly documenting/handling the changed semantics to avoid regressions.
- In v4l2core_prepare_valid_resolution, when no valid resolution is found you log an error but still leave my_width/my_height at 0; it would be safer to either retain the previous valid values, fall back to a known safe default, or propagate an explicit failure rather than continuing with a 0x0 resolution.
- The new preferred resolution helpers in v4l2_formats.c use printf for logging, which is inconsistent with the rest of the codebase’s logging style; consider switching to the existing logging mechanism (e.g. qInfo/qWarning or project-specific macros) for uniform output and better integration with log handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new resolution selection path in v4l2core_prepare_valid_resolution now ignores the previous my_width/my_height-based logic and always chooses either the preferred or the max resolution; consider preserving the old behavior (e.g. respecting an initial target resolution and the 3840x2160 limit) or explicitly documenting/handling the changed semantics to avoid regressions.
- In v4l2core_prepare_valid_resolution, when no valid resolution is found you log an error but still leave my_width/my_height at 0; it would be safer to either retain the previous valid values, fall back to a known safe default, or propagate an explicit failure rather than continuing with a 0x0 resolution.
- The new preferred resolution helpers in v4l2_formats.c use printf for logging, which is inconsistent with the rest of the codebase’s logging style; consider switching to the existing logging mechanism (e.g. qInfo/qWarning or project-specific macros) for uniform output and better integration with log handling.
## Individual Comments
### Comment 1
<location path="libcam/libcam_v4l2core/v4l2_core.c" line_range="1706-1715" />
<code_context>
+ for (int i = 0; i < vd->list_stream_formats[format_index].numb_res; i++)
</code_context>
<issue_to_address>
**issue:** Handle the case where no valid resolution is found more robustly.
Currently, if no valid resolution is found, `max_width`/`max_height` stay at 0, causing `my_width`/`my_height` to be set to 0 and only logging an error. If callers treat these as valid, this can result in invalid downstream configuration. Please either fall back to the previous `my_width`/`my_height`, return/propagate an error from this function, or ensure that a `(0, 0)` resolution is never used later in the pipeline.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1706
to
+1715
| for (int i = 0; i < vd->list_stream_formats[format_index].numb_res; i++) | ||
| { | ||
| int w = vd->list_stream_formats[format_index].list_stream_cap[i].width; | ||
| int h = vd->list_stream_formats[format_index].list_stream_cap[i].height; | ||
| printf("V4L2_CORE: - valid resolution(%dx%d)\n", w, h); | ||
| if (my_width <= w && my_height <= h && is_valid_resolution(w, h)) { | ||
| my_width = w; | ||
| my_height = h; | ||
| if (is_valid_resolution(w, h)) | ||
| { | ||
| // 找出首选分辨率 | ||
| printf("V4L2_CORE: check valid resolution, width: %d, height: %d, preferred width: %d, preferred height: %d\n", | ||
| w, h, pref_w, pref_h); | ||
| if (!found_preferred && pref_w != 0 && pref_h != 0 && w == pref_w && h == pref_h) { |
There was a problem hiding this comment.
issue: Handle the case where no valid resolution is found more robustly.
Currently, if no valid resolution is found, max_width/max_height stay at 0, causing my_width/my_height to be set to 0 and only logging an error. If callers treat these as valid, this can result in invalid downstream configuration. Please either fall back to the previous my_width/my_height, return/propagate an error from this function, or ensure that a (0, 0) resolution is never used later in the pipeline.
Log: 支持通过配置标记低性能设备 Bug: https://pms.uniontech.com/bug-view-358869.html Influence: 可通过配置直接识别低性能设备，优化低性能设备判断逻辑。
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // v4l2_core.c 中的改进建议
if (found_preferred) {
// 有首选分辨率，使用首选分辨率
my_width = pref_w;
my_height = pref_h;
} else {
// 没有首选分辨率，使用最大分辨率
my_width = max_width;
my_height = max_height;
// 极端情况，没有可用分辨率，发出错误提示并设置默认值
if (max_width == 0 || max_height == 0) {
fprintf(stderr, "V4L2_CORE: ERROR: no valid resolution found!\n");
// 设置一个安全的默认分辨率，防止后续流程崩溃
my_width = 640;
my_height = 480;
}
} |
lzwind
approved these changes
Jul 21, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, lzwind 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
4 participants
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.
Summary by Sourcery
Integrate configurable preferred camera resolution and low-performance device detection into the camera pipeline and configuration handling.
New Features:
Enhancements: