fix: Add DConfig for preview with no delay.#460
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 12, 2026
Merged
fix: Add DConfig for preview with no delay.#460deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a new DataManager flag controlled by DConfig to optionally disable the preview throttling sleep, enabling a no-delay preview mode when configured. Sequence diagram for DConfig controlled preview no delay behaviorsequenceDiagram
participant Main
participant DConfig
participant DataManager
participant MajorImageProcessingThread
Main->>DConfig: load()
alt previewNoDelay key exists and config is valid
Main->>DConfig: value(previewNoDelay)
DConfig-->>Main: bool previewNoDelay
Main->>DataManager: setPreviewNoDelay(previewNoDelay)
end
MajorImageProcessingThread->>DataManager: isPreviewNoDelay()
DataManager-->>MajorImageProcessingThread: bool previewNoDelay
alt previewNoDelay is false and m_nVdWidth <= 1920
MajorImageProcessingThread->>MajorImageProcessingThread: msleep(33)
else previewNoDelay is true or m_nVdWidth > 1920
MajorImageProcessingThread->>MajorImageProcessingThread: process frame without sleep
end
Class diagram for updated DataManager preview no delay flagclassDiagram
class DataManager {
- static DataManager* m_dataManager
- volatile DeviceStatus m_devStatus
- bool m_H264EncoderExists
- bool m_isSupportCameraSwitch
- bool m_isPreviewNoDelay
+ DataManager* instance()
+ bool isSupportCameraSwitch()
+ void setIsSupportCameraSwitch(bool isTrue)
+ void setPreviewNoDelay(bool isTrue)
+ bool isPreviewNoDelay()
}
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:
- Consider marking
isPreviewNoDelay()(andisSupportCameraSwitch()while you're here) asconstto better express intent and allow use on constDataManagerinstances. - Access to
m_isPreviewNoDelayfrom the processing thread goes throughDataManager::instance(); ifDataManageris touched from multiple threads, consider making this flag atomic or otherwise thread-safe to avoid data races. - The DConfig read for
previewNoDelaycan be simplified by skipping thekeyList().contains()check and just using a default value withdconfig->value("previewNoDelay", false).toBool()to reduce lookups and keep the logic consistent with other keys.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider marking `isPreviewNoDelay()` (and `isSupportCameraSwitch()` while you're here) as `const` to better express intent and allow use on const `DataManager` instances.
- Access to `m_isPreviewNoDelay` from the processing thread goes through `DataManager::instance()`; if `DataManager` is touched from multiple threads, consider making this flag atomic or otherwise thread-safe to avoid data races.
- The DConfig read for `previewNoDelay` can be simplified by skipping the `keyList().contains()` check and just using a default value with `dconfig->value("previewNoDelay", false).toBool()` to reduce lookups and keep the logic consistent with other keys.
## Individual Comments
### Comment 1
<location path="src/src/basepub/datamanager.h" line_range="178" />
<code_context>
volatile enum DeviceStatus m_devStatus;
bool m_H264EncoderExists;
bool m_isSupportCameraSwitch = false; // 是否带有摄像头开关
+ bool m_isPreviewNoDelay = false; // 是否预览无延迟
};
#endif // DATAMANAGER_H
</code_context>
<issue_to_address>
**issue (bug_risk):** Access to `m_isPreviewNoDelay` from multiple threads may need synchronization or an atomic type.
This flag is written from the main thread (`setPreviewNoDelay`) and read from `MajorImageProcessingThread::run()`. If those can overlap, this is a data race in standard C++. Please make it `std::atomic<bool>` or protect it with the same synchronization used for other shared `DataManager` state.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Add DConfig for preview with no delay. DConfig中增加关于预览无延迟的配置。 Bug: https://pms.uniontech.com/bug-view-351345.html v20 BUG 分支合一到v25主线 Task: https://pms.uniontech.com/task-view-383481.html
deepin pr auto reviewGit Diff 代码审查报告我已仔细审查了提供的 Git diff,以下是我的分析和改进建议: 整体评价这段代码添加了一个"预览无延迟"功能,通过配置项控制是否在视频预览时降低刷新率。代码实现了基本功能,但在一些细节上可以进一步优化。 详细审查1. 语法逻辑优点:
问题:
2. 代码质量优点:
问题:
3. 代码性能优点:
问题:
4. 代码安全优点:
问题:
改进建议
改进后的代码示例// org.deepin.camera.encode.json
"previewNoDelay": {
"value": false,
"serial": 1, // 使用唯一值
"flags": ["global"],
"name": "preview no delay",
"name[zh_CN]": "是否预览无延迟",
"description": "Whether to disable preview delay for real-time effect",
"permissions": "readwrite",
"visibility": "private"
}// datamanager.h
class DataManager: public QObject
{
Q_OBJECT
public:
/**
* @brief 设置是否预览无延迟
* @param enable 是否启用无延迟模式
*/
void setPreviewNoDelay(bool enable) {
QMutexLocker locker(&m_mutex);
m_isPreviewNoDelay = enable;
};
/**
* @brief 获取是否预览无延迟
* @return 是否启用无延迟模式
*/
bool isPreviewNoDelay() const {
QMutexLocker locker(&m_mutex);
return m_isPreviewNoDelay;
};
private:
mutable QMutex m_mutex;
bool m_isPreviewNoDelay = false; // 是否预览无延迟
};// majorimageprocessingthread.cpp
void MajorImageProcessingThread::run()
{
// ...
// 保证画面流畅的前提下降低刷新率
// 无延迟模式下可能增加CPU/GPU负载,某些客户要求更加实时的效果会使用此配置。
if (!DataManager::instance()->isPreviewNoDelay() && m_nVdWidth <= 1920) {
msleep(33);
}
// ...
}// main.cpp
int main(int argc, char *argv[])
{
// ...
if (dconfig && dconfig->isValid()) {
if (dconfig->keyList().contains("supportCameraSwitch")) {
DataManager::instance()->setIsSupportCameraSwitch(dconfig->value("supportCameraSwitch").toBool());
}
if (dconfig->keyList().contains("previewNoDelay")) {
DataManager::instance()->setPreviewNoDelay(dconfig->value("previewNoDelay", false).toBool());
}
}
// ...
}这些改进将使代码更加健壮、安全和可维护,同时保持原有功能不变。 |
lzwind
approved these changes
Mar 12, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, 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
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 for preview with no delay.
DConfig中增加关于预览无延迟的配置。
Bug: https://pms.uniontech.com/bug-view-351345.html
v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html
Summary by Sourcery
Add configuration-driven support to toggle preview no-delay behavior and wire it into the main data manager and image processing thread.
New Features:
Enhancements: