feat(config): add useRgbData config to control RGB preview mode#488
Merged
Merged
Conversation
Reviewer's GuideAdds a new DConfig-driven switch to control whether RGB data is used for preview rendering, wiring it through DataManager, main startup configuration, and the major image processing thread’s RGB path selection logic. Sequence diagram for DConfig-driven RGB preview mode selectionsequenceDiagram
participant Main
participant DConfig
participant DataManager
participant MajorImageProcessingThread
Main->>DConfig: value(useRgbData)
DConfig-->>Main: int useRgbData
Main->>DataManager: setUseRgbData(useRgbData)
Main-->>MajorImageProcessingThread: start()
MajorImageProcessingThread->>DataManager: getUseRgbData()
DataManager-->>MajorImageProcessingThread: useRgbData
alt [useRgbData == 1]
MajorImageProcessingThread->>MajorImageProcessingThread: bUseRgb = true
else [useRgbData == 0]
MajorImageProcessingThread->>MajorImageProcessingThread: bUseRgb = false
else [useRgbData == -1]
MajorImageProcessingThread->>MajorImageProcessingThread: [system auto decides bUseRgb]
end
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 2 issues, and left some high level feedback:
- Consider replacing the raw
intvalues-1/0/1foruseRgbDatawith a strongly typed enum to make the meaning of each mode clearer at the call site and reduce the risk of invalid values. - It may be worth validating/clamping the
useRgbDatavalue read from DConfig inmain.cpp(e.g., restricting to -1, 0, 1) before storing it inDataManagerto avoid unexpected behavior from malformed configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider replacing the raw `int` values `-1/0/1` for `useRgbData` with a strongly typed enum to make the meaning of each mode clearer at the call site and reduce the risk of invalid values.
- It may be worth validating/clamping the `useRgbData` value read from DConfig in `main.cpp` (e.g., restricting to -1, 0, 1) before storing it in `DataManager` to avoid unexpected behavior from malformed configuration.
## Individual Comments
### Comment 1
<location path="src/main.cpp" line_range="220-224" />
<code_context>
}
}
+ if (dconfig && dconfig->isValid() && dconfig->keyList().contains("useRgbData")) {
+ int useRgbData = dconfig->value("useRgbData").toInt();
+ qInfo() << "use RGB data for preview:" << useRgbData;
+ DataManager::instance()->setUseRgbData(useRgbData);
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Validate or clamp the configuration value for `useRgbData` before storing it.
`useRgbData` is documented as `-1/0/1`, but any value from the config is forwarded directly. If an invalid or legacy value is present, it will be treated as "auto" in `MajorImageProcessingThread`, which can be misleading when debugging. Please validate and either clamp or map unknown values to a default (e.g. `-1`) before calling `setUseRgbData`.
```suggestion
if (dconfig && dconfig->isValid() && dconfig->keyList().contains("useRgbData")) {
int useRgbData = dconfig->value("useRgbData").toInt();
// Normalize to supported values -1 (auto), 0 (disabled), 1 (enabled)
if (useRgbData != -1 && useRgbData != 0 && useRgbData != 1) {
qWarning() << "Invalid useRgbData value in config:" << useRgbData
<< "- mapping to default value -1 (auto)";
useRgbData = -1;
}
qInfo() << "use RGB data for preview:" << useRgbData;
DataManager::instance()->setUseRgbData(useRgbData);
}
```
</issue_to_address>
### Comment 2
<location path="src/src/majorimageprocessingthread.cpp" line_range="286-289" />
<code_context>
bUseRgb = true;
+ // DConfig配置控制RGB数据使用模式，-1表示不设置(由系统自动判断)，0表示强制关闭，1表示强制开启
+ int useRgbData = DataManager::instance()->getUseRgbData();
+ if (useRgbData == 1) {
+ bUseRgb = true;
+ } else if (useRgbData == 0) {
+ bUseRgb = false;
+ }
</code_context>
<issue_to_address>
**suggestion:** Clarify handling of unexpected `useRgbData` values to make the auto behavior explicit.
Right now only `0`/`1` change `bUseRgb`; any other value (including the documented `-1` auto) just falls back to the old behavior. If the intent is “non-0/1 = auto”, consider encoding that explicitly (e.g. switch or explicit `-1` case), and/or logging/asserting on unexpected values to surface config mistakes early.
Suggested implementation:
```cpp
// DConfig配置控制RGB数据使用模式，-1表示不设置(由系统自动判断)，0表示强制关闭，1表示强制开启
int useRgbData = DataManager::instance()->getUseRgbData();
switch (useRgbData) {
case 1:
// 强制开启 RGB
bUseRgb = true;
break;
case 0:
// 强制关闭 RGB
bUseRgb = false;
break;
case -1:
// 自动模式：保持当前 bUseRgb 由系统逻辑决定
break;
default:
// 非法配置值：保持当前 bUseRgb，同时在开发期暴露问题
assert(false && "Invalid useRgbData configuration value (expected -1/0/1)");
break;
}
```
1. Ensure `<cassert>` (or your project’s standard assert/logging header) is included in `majorimageprocessingthread.cpp` or a common header; e.g. add `#include <cassert>` if not already present.
2. If your codebase uses a specific logging/assert macro (e.g. `LOG_WARNING`, `Q_ASSERT`, etc.), replace the `assert(false && "...")` call with that preferred mechanism to align with existing conventions.
</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
+286
to
+289
| int useRgbData = DataManager::instance()->getUseRgbData(); | ||
| if (useRgbData == 1) { | ||
| bUseRgb = true; | ||
| } else if (useRgbData == 0) { |
There was a problem hiding this comment.
suggestion: Clarify handling of unexpected useRgbData values to make the auto behavior explicit.
Right now only 0/1 change bUseRgb; any other value (including the documented -1 auto) just falls back to the old behavior. If the intent is “non-0/1 = auto”, consider encoding that explicitly (e.g. switch or explicit -1 case), and/or logging/asserting on unexpected values to surface config mistakes early.
Suggested implementation:
// DConfig配置控制RGB数据使用模式，-1表示不设置(由系统自动判断)，0表示强制关闭，1表示强制开启
int useRgbData = DataManager::instance()->getUseRgbData();
switch (useRgbData) {
case 1:
// 强制开启 RGB
bUseRgb = true;
break;
case 0:
// 强制关闭 RGB
bUseRgb = false;
break;
case -1:
// 自动模式：保持当前 bUseRgb 由系统逻辑决定
break;
default:
// 非法配置值：保持当前 bUseRgb，同时在开发期暴露问题
assert(false && "Invalid useRgbData configuration value (expected -1/0/1)");
break;
}
- Ensure
<cassert>(or your project’s standard assert/logging header) is included inmajorimageprocessingthread.cppor a common header; e.g. add#include <cassert>if not already present. - If your codebase uses a specific logging/assert macro (e.g.
LOG_WARNING,Q_ASSERT, etc.), replace theassert(false && "...")call with that preferred mechanism to align with existing conventions.
Add DConfig option "useRgbData" to control whether YUV frames are converted to RGB for preview rendering. Supports three modes: -1: auto (default, let system decide) 0: force disable 1: force enable 添加DConfig配置项"useRgbData"，控制预览时是否将YUV帧数据转换为RGB格式。 支持三态：-1自动判断（默认）、0强制关闭、1强制开启。 Log: 添加RGB预览模式配置项 PMS: BUG-366739 Influence: 通过DConfig配置可强制控制预览RGB数据的使用，便于排查特定机型显示异常问题。
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 以下为可选的性能微优化示例，将配置读取移出 run() 循环体外部（仅在初始化时读取一次）
// 文件：src/src/majorimageprocessingthread.cpp
void MajorImageProcessingThread::run()
{
// 在线程启动时读取一次配置，避免在每帧处理的循环中重复调用
const int useRgbData = DataManager::instance()->getUseRgbData();
while (m_bRunning) {
// ...
// 判断是否使用rgb数据
bool bUseRgb = false;
#if defined(_loongarch) || defined(__loongarch__) || defined(__loongarch64) || defined (__mips__)
bUseRgb = true;
#endif
if(DSysInfo::majorVersion().toInt() >= 23) {
bUseRgb = true;
}
if (GStreamer_Env == m_eEncodeEnv)
bUseRgb = true;
// DConfig配置控制RGB数据使用模式，-1表示不设置(由系统自动判断)，0表示强制关闭，1表示强制开启
if (useRgbData == 1) {
bUseRgb = true;
} else if (useRgbData == 0) {
bUseRgb = false;
}
// ...
}
} |
lzwind
approved these changes
Jun 24, 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
3 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.
Add DConfig option "useRgbData" to control whether YUV frames are converted to RGB for preview rendering. Supports three modes: -1: auto (default, let system decide)
0: force disable
1: force enable
添加DConfig配置项"useRgbData"，控制预览时是否将YUV帧数据转换为RGB格式。
支持三态：-1自动判断（默认）、0强制关闭、1强制开启。
Log: 添加RGB预览模式配置项
PMS: BUG-366739
Influence: 通过DConfig配置可强制控制预览RGB数据的使用，便于排查特定机型显示异常问题。
Summary by Sourcery
Add a configurable RGB preview mode controlled by a new DConfig option.
New Features:
Enhancements: