feat(display): support video wallpaper thumbnail in monitor schematic#3323
Open
deepin-wm wants to merge 1 commit into
Open
feat(display): support video wallpaper thumbnail in monitor schematic#3323deepin-wm wants to merge 1 commit into
deepin-wm wants to merge 1 commit into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-wm 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 |
Reviewer's GuideSupport video wallpapers in the monitor schematic by resolving and displaying thumbnail images instead of video file paths, with asynchronous thumbnail generation and caching for both Wayland and X11 paths. Sequence diagram for asynchronous video wallpaper thumbnail resolutionsequenceDiagram
participant DisplayWorker
participant Monitor
participant QtConcurrent
participant FfmpegThumbnailer
DisplayWorker->>DisplayWorker: updateMonitorWallpaper(mon)
DisplayWorker->>DisplayWorker: resolveVideoThumbnail(videoPath, mon)
alt metadata.json or cache hit
DisplayWorker->>Monitor: setWallpaper(thumbnailPath)
else no cached thumbnail
DisplayWorker->>DisplayWorker: m_videoWallpaperMonitors[videoPath] = monitor
DisplayWorker->>QtConcurrent: QtConcurrent::run(lambda(videoPath, cachePath))
QtConcurrent->>FfmpegThumbnailer: generateThumbnail(videoPath, Png, cachePath)
FfmpegThumbnailer-->>DisplayWorker: videoThumbnailReady(videoPath, cachePath)
DisplayWorker->>Monitor: setWallpaper(thumbnailPath)
DisplayWorker->>DisplayWorker: m_videoWallpaperMonitors.remove(videoPath)
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 3 issues, and left some high level feedback:
- Consider caching and reusing the parsed metadata.json for SysLiveWallpaperDir instead of re-reading and reparsing it on every resolveVideoThumbnail() call to avoid repeated disk IO and JSON parsing.
- isVideoFile() relies purely on file suffix with a hardcoded list; you may want to centralize/extend this (e.g., via QMimeType or a configurable list) to avoid missing supported formats or duplicating logic elsewhere.
- resolveVideoThumbnail() recreates the cache directory and computes the cache path on each call; factoring out cache directory initialization and cache name generation into helpers can simplify the function and reduce repeated work.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider caching and reusing the parsed metadata.json for SysLiveWallpaperDir instead of re-reading and reparsing it on every resolveVideoThumbnail() call to avoid repeated disk IO and JSON parsing.
- isVideoFile() relies purely on file suffix with a hardcoded list; you may want to centralize/extend this (e.g., via QMimeType or a configurable list) to avoid missing supported formats or duplicating logic elsewhere.
- resolveVideoThumbnail() recreates the cache directory and computes the cache path on each call; factoring out cache directory initialization and cache name generation into helpers can simplify the function and reduce repeated work.
## Individual Comments
### Comment 1
<location path="src/plugin-display/operation/private/displayworker.cpp" line_range="1393-1396" />
<code_context>
+ }
+ }
+
+ QString cacheDir = QStandardPaths::writableLocation(QStandardPaths::CacheLocation) + "/live-wallpaper-thumbnails";
+ QDir().mkpath(cacheDir);
+ QString cacheName = QString(QCryptographicHash::hash(videoPath.toUtf8(), QCryptographicHash::Md5).toHex()) + ".png";
+ QString cachePath = cacheDir + "/" + cacheName;
+
+ if (QFile::exists(cachePath)) {
</code_context>
<issue_to_address>
**issue:** Cache directory handling doesn’t check for failure and may behave unexpectedly if the location is empty or invalid.
If `QStandardPaths::writableLocation(CacheLocation)` returns an empty string or a path that can’t be created, `QDir().mkpath(cacheDir)` will fail silently, and `cachePath` may point to an unusable or unintended location. Please verify that `cacheDir` is non-empty and that `mkpath` returns true before using it, and fall back to `videoPath` or another safe default if the cache location isn’t usable.
</issue_to_address>
### Comment 2
<location path="src/plugin-display/operation/private/displayworker.cpp" line_range="1402-1406" />
<code_context>
+ return cachePath;
+ }
+
+ if (m_videoWallpaperMonitors.contains(videoPath)) {
+ return videoPath;
+ }
+
+ m_videoWallpaperMonitors[videoPath] = monitor;
+
+ QPointer<DisplayWorker> guard(this);
</code_context>
<issue_to_address>
**issue (bug_risk):** Video thumbnail mapping only supports a single monitor per video path, which may break multi-monitor setups.
Keying `m_videoWallpaperMonitors` by `videoPath` causes later assignments to overwrite earlier ones when the same video is used on multiple monitors, so only one monitor gets updated when thumbnail generation completes. If sharing video wallpapers across monitors is intended, map each `videoPath` to multiple monitors (e.g. a `QList<QPointer<Monitor>>` or similar collection) instead.
</issue_to_address>
### Comment 3
<location path="src/plugin-display/operation/private/displayworker.cpp" line_range="1363" />
<code_context>
m_model->setIsConcatScreenMode(m_displayInter->isConcatScreenEnabled());
}
+
+QString DisplayWorker::resolveVideoThumbnail(const QString &videoPath, Monitor *monitor)
+{
+ QDir liveDir(SysLiveWallpaperDir);
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new video wallpaper handling into shared helpers that hide thumbnail resolution details and isolate state management around `m_videoWallpaperMonitors`.
The added video handling is functionally fine but does increase complexity in three separate call sites and in `resolveVideoThumbnail`. You can keep the feature intact while reducing cognitive load by:
1. Centralizing “wallpaper vs video thumbnail” resolution
2. Splitting `resolveVideoThumbnail` into smaller helpers (metadata lookup vs async generation)
3. Narrowing where `m_videoWallpaperMonitors` is touched
### 1. Centralize wallpaper resolution
Instead of branching in each caller, introduce a small helper that hides video-specific logic:
```cpp
// in DisplayWorker.h
private:
QString resolveWallpaperForMonitor(Monitor *mon,
const QString &fileSource,
uint32_t sourceType = 0);
// in DisplayWorker.cpp
QString DisplayWorker::resolveWallpaperForMonitor(Monitor *mon,
const QString &fileSource,
uint32_t sourceType)
{
const QString path = fileSource.isEmpty()
? m_displayInter->GetCurrentWorkspaceBackgroundForMonitor(mon->name())
: fileSource;
const bool isVideo = (sourceType == WallpaperSourceTypeVideo) || isVideoFile(path);
return isVideo ? resolveVideoThumbnail(path, mon) : path;
}
```
Then use this helper in the three call sites:
```cpp
void DisplayWorker::updateMonitorWallpaper(Monitor *mon)
{
mon->setWallpaper(resolveWallpaperForMonitor(mon, QString{}));
}
void DisplayWorker::onOutputWallpaperReady(WQt::Output *output)
{
...
if (wp) {
connect(...);
if (wp->sourceType() == WallpaperSourceTypeVideo && !wp->fileSource().isEmpty()) {
for (auto it = m_wl_monitors.cbegin(); it != m_wl_monitors.cend(); ++it) {
if (it.key()->name() == output->name()) {
it.key()->setWallpaper(resolveWallpaperForMonitor(it.key(),
wp->fileSource(),
wp->sourceType()));
break;
}
}
}
}
}
void DisplayWorker::onWallpaperChanged(const QString &fileSource,
uint32_t sourceType,
uint32_t role)
{
Q_UNUSED(role);
...
for (auto it = m_wl_monitors.cbegin(); it != m_wl_monitors.cend(); ++it) {
if (it.key()->name() == outputName) {
it.key()->setWallpaper(resolveWallpaperForMonitor(it.key(),
fileSource,
sourceType));
break;
}
}
}
```
This keeps the existing methods mostly unaware of video-specific details.
### 2. Split `resolveVideoThumbnail` responsibilities
`resolveVideoThumbnail` currently mixes JSON parsing, cache management, async job scheduling, and monitor bookkeeping. Splitting into tiny helpers makes it easier to follow and test:
```cpp
// in DisplayWorker.h
private:
QString findLiveWallpaperThumbnail(const QString &videoPath) const;
QString ensureThumbnailCachePath(const QString &videoPath) const;
void scheduleThumbnailGeneration(const QString &videoPath,
const QString &cachePath,
Monitor *monitor);
```
Implementation:
```cpp
QString DisplayWorker::findLiveWallpaperThumbnail(const QString &videoPath) const
{
QDir liveDir(SysLiveWallpaperDir);
if (!liveDir.exists())
return {};
QFile metaFile(liveDir.absoluteFilePath("metadata.json"));
if (!metaFile.open(QIODevice::ReadOnly))
return {};
QJsonParseError parseErr;
const QJsonDocument doc = QJsonDocument::fromJson(metaFile.readAll(), &parseErr);
if (parseErr.error != QJsonParseError::NoError || !doc.isArray())
return {};
for (const auto &entry : doc.array()) {
const QJsonObject obj = entry.toObject();
const QString videoAbsPath = liveDir.absoluteFilePath(obj.value("path").toString());
if (videoAbsPath == videoPath) {
const QString thumbnailRel = obj.value("thumbnail").toString();
const QString thumbAbsPath = liveDir.absoluteFilePath(thumbnailRel);
if (!thumbnailRel.isEmpty() && QFile::exists(thumbAbsPath))
return thumbAbsPath;
break;
}
}
return {};
}
QString DisplayWorker::ensureThumbnailCachePath(const QString &videoPath) const
{
const QString cacheDir =
QStandardPaths::writableLocation(QStandardPaths::CacheLocation)
+ "/live-wallpaper-thumbnails";
QDir().mkpath(cacheDir);
const QString cacheName =
QString(QCryptographicHash::hash(videoPath.toUtf8(),
QCryptographicHash::Md5).toHex())
+ ".png";
return cacheDir + "/" + cacheName;
}
void DisplayWorker::scheduleThumbnailGeneration(const QString &videoPath,
const QString &cachePath,
Monitor *monitor)
{
m_videoWallpaperMonitors[videoPath] = monitor;
QPointer<DisplayWorker> guard(this);
(void)QtConcurrent::run([guard, videoPath, cachePath]() {
ffmpegthumbnailer::VideoThumbnailer thumbnailer(480, false, true, 8, false);
thumbnailer.setThumbnailSize(480, -1);
thumbnailer.setSeekTime("00:00:01");
try {
thumbnailer.generateThumbnail(videoPath.toStdString(),
Png,
cachePath.toStdString());
} catch (const std::exception &e) {
qCWarning(DdcDisplayWorker) << "Failed to generate video thumbnail:" << e.what();
if (guard)
Q_EMIT guard->videoThumbnailReady(videoPath, {});
return;
}
if (guard && QFile::exists(cachePath))
Q_EMIT guard->videoThumbnailReady(videoPath, cachePath);
});
}
```
Now `resolveVideoThumbnail` becomes a simple orchestration method:
```cpp
QString DisplayWorker::resolveVideoThumbnail(const QString &videoPath, Monitor *monitor)
{
if (const QString liveThumb = findLiveWallpaperThumbnail(videoPath); !liveThumb.isEmpty())
return liveThumb;
const QString cachePath = ensureThumbnailCachePath(videoPath);
if (QFile::exists(cachePath))
return cachePath;
if (!m_videoWallpaperMonitors.contains(videoPath))
scheduleThumbnailGeneration(videoPath, cachePath, monitor);
// fall back to video path until thumbnail is ready
return videoPath;
}
```
This keeps filesystem/JSON logic, cache naming, and async job management clearly separated.
### 3. Narrow `m_videoWallpaperMonitors` usage
With the above split, `m_videoWallpaperMonitors` is only touched in:
- `scheduleThumbnailGeneration` (insert)
- the `videoThumbnailReady` lambda in the constructor (lookup + remove)
No other code paths need to know about this map, making the monitor update flow easier to track and reducing mutable state spread inside the class.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| m_model->setIsConcatScreenMode(m_displayInter->isConcatScreenEnabled()); | ||
| } | ||
|
|
||
| QString DisplayWorker::resolveVideoThumbnail(const QString &videoPath, Monitor *monitor) |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the new video wallpaper handling into shared helpers that hide thumbnail resolution details and isolate state management around m_videoWallpaperMonitors.
The added video handling is functionally fine but does increase complexity in three separate call sites and in resolveVideoThumbnail. You can keep the feature intact while reducing cognitive load by:
- Centralizing “wallpaper vs video thumbnail” resolution
- Splitting
resolveVideoThumbnailinto smaller helpers (metadata lookup vs async generation) - Narrowing where
m_videoWallpaperMonitorsis touched
1. Centralize wallpaper resolution
Instead of branching in each caller, introduce a small helper that hides video-specific logic:
// in DisplayWorker.h
private:
QString resolveWallpaperForMonitor(Monitor *mon,
const QString &fileSource,
uint32_t sourceType = 0);
// in DisplayWorker.cpp
QString DisplayWorker::resolveWallpaperForMonitor(Monitor *mon,
const QString &fileSource,
uint32_t sourceType)
{
const QString path = fileSource.isEmpty()
? m_displayInter->GetCurrentWorkspaceBackgroundForMonitor(mon->name())
: fileSource;
const bool isVideo = (sourceType == WallpaperSourceTypeVideo) || isVideoFile(path);
return isVideo ? resolveVideoThumbnail(path, mon) : path;
}Then use this helper in the three call sites:
void DisplayWorker::updateMonitorWallpaper(Monitor *mon)
{
mon->setWallpaper(resolveWallpaperForMonitor(mon, QString{}));
}
void DisplayWorker::onOutputWallpaperReady(WQt::Output *output)
{
...
if (wp) {
connect(...);
if (wp->sourceType() == WallpaperSourceTypeVideo && !wp->fileSource().isEmpty()) {
for (auto it = m_wl_monitors.cbegin(); it != m_wl_monitors.cend(); ++it) {
if (it.key()->name() == output->name()) {
it.key()->setWallpaper(resolveWallpaperForMonitor(it.key(),
wp->fileSource(),
wp->sourceType()));
break;
}
}
}
}
}
void DisplayWorker::onWallpaperChanged(const QString &fileSource,
uint32_t sourceType,
uint32_t role)
{
Q_UNUSED(role);
...
for (auto it = m_wl_monitors.cbegin(); it != m_wl_monitors.cend(); ++it) {
if (it.key()->name() == outputName) {
it.key()->setWallpaper(resolveWallpaperForMonitor(it.key(),
fileSource,
sourceType));
break;
}
}
}This keeps the existing methods mostly unaware of video-specific details.
2. Split resolveVideoThumbnail responsibilities
resolveVideoThumbnail currently mixes JSON parsing, cache management, async job scheduling, and monitor bookkeeping. Splitting into tiny helpers makes it easier to follow and test:
// in DisplayWorker.h
private:
QString findLiveWallpaperThumbnail(const QString &videoPath) const;
QString ensureThumbnailCachePath(const QString &videoPath) const;
void scheduleThumbnailGeneration(const QString &videoPath,
const QString &cachePath,
Monitor *monitor);Implementation:
QString DisplayWorker::findLiveWallpaperThumbnail(const QString &videoPath) const
{
QDir liveDir(SysLiveWallpaperDir);
if (!liveDir.exists())
return {};
QFile metaFile(liveDir.absoluteFilePath("metadata.json"));
if (!metaFile.open(QIODevice::ReadOnly))
return {};
QJsonParseError parseErr;
const QJsonDocument doc = QJsonDocument::fromJson(metaFile.readAll(), &parseErr);
if (parseErr.error != QJsonParseError::NoError || !doc.isArray())
return {};
for (const auto &entry : doc.array()) {
const QJsonObject obj = entry.toObject();
const QString videoAbsPath = liveDir.absoluteFilePath(obj.value("path").toString());
if (videoAbsPath == videoPath) {
const QString thumbnailRel = obj.value("thumbnail").toString();
const QString thumbAbsPath = liveDir.absoluteFilePath(thumbnailRel);
if (!thumbnailRel.isEmpty() && QFile::exists(thumbAbsPath))
return thumbAbsPath;
break;
}
}
return {};
}
QString DisplayWorker::ensureThumbnailCachePath(const QString &videoPath) const
{
const QString cacheDir =
QStandardPaths::writableLocation(QStandardPaths::CacheLocation)
+ "/live-wallpaper-thumbnails";
QDir().mkpath(cacheDir);
const QString cacheName =
QString(QCryptographicHash::hash(videoPath.toUtf8(),
QCryptographicHash::Md5).toHex())
+ ".png";
return cacheDir + "/" + cacheName;
}
void DisplayWorker::scheduleThumbnailGeneration(const QString &videoPath,
const QString &cachePath,
Monitor *monitor)
{
m_videoWallpaperMonitors[videoPath] = monitor;
QPointer<DisplayWorker> guard(this);
(void)QtConcurrent::run([guard, videoPath, cachePath]() {
ffmpegthumbnailer::VideoThumbnailer thumbnailer(480, false, true, 8, false);
thumbnailer.setThumbnailSize(480, -1);
thumbnailer.setSeekTime("00:00:01");
try {
thumbnailer.generateThumbnail(videoPath.toStdString(),
Png,
cachePath.toStdString());
} catch (const std::exception &e) {
qCWarning(DdcDisplayWorker) << "Failed to generate video thumbnail:" << e.what();
if (guard)
Q_EMIT guard->videoThumbnailReady(videoPath, {});
return;
}
if (guard && QFile::exists(cachePath))
Q_EMIT guard->videoThumbnailReady(videoPath, cachePath);
});
}Now resolveVideoThumbnail becomes a simple orchestration method:
QString DisplayWorker::resolveVideoThumbnail(const QString &videoPath, Monitor *monitor)
{
if (const QString liveThumb = findLiveWallpaperThumbnail(videoPath); !liveThumb.isEmpty())
return liveThumb;
const QString cachePath = ensureThumbnailCachePath(videoPath);
if (QFile::exists(cachePath))
return cachePath;
if (!m_videoWallpaperMonitors.contains(videoPath))
scheduleThumbnailGeneration(videoPath, cachePath, monitor);
// fall back to video path until thumbnail is ready
return videoPath;
}This keeps filesystem/JSON logic, cache naming, and async job management clearly separated.
3. Narrow m_videoWallpaperMonitors usage
With the above split, m_videoWallpaperMonitors is only touched in:
scheduleThumbnailGeneration(insert)- the
videoThumbnailReadylambda in the constructor (lookup + remove)
No other code paths need to know about this map, making the monitor update flow easier to track and reducing mutable state spread inside the class.
1. Add resolveVideoThumbnail() to DisplayWorker for video wallpaper 2. Resolve thumbnail from metadata.json, ffmpegthumbnailer cache, or async generation via QtConcurrent 3. Add isVideoFile() helper and WallpaperSourceTypeVideo constant 4. Handle video wallpaper in both Wayland and X11 code paths 5. Add Qt6::Concurrent and libffmpegthumbnailer CMake dependencies Log: Monitor schematic now shows video wallpaper thumbnails correctly Influence: 1. Test monitor schematic display with static wallpaper 2. Test monitor schematic display with video wallpaper 3. Verify thumbnail generation for various video formats 4. Test multi-monitor setup with different wallpaper types 5. Verify async thumbnail generation completes and updates display feat(display): 显示器示意图支持视频壁纸缩略图 1. 在 DisplayWorker 中添加 resolveVideoThumbnail() 方法 2. 从 metadata.json、ffmpegthumbnailer 缓存或异步生成获取缩略图 3. 添加 isVideoFile() 辅助函数和 WallpaperSourceTypeVideo 常量 4. 在 Wayland 和 X11 两条路径中处理视频壁纸 5. 添加 Qt6::Concurrent 和 libffmpegthumbnailer CMake 依赖 Log: 显示器示意图可正常显示视频壁纸缩略图 Influence: 1. 测试静态壁纸下显示器示意图的显示 2. 测试视频壁纸下显示器示意图的显示 3. 验证各种视频格式的缩略图生成 4. 测试多显示器不同壁纸类型的配置 5. 验证异步缩略图生成完成后显示更新 PMS: BUG-367369
deepin-wm
force-pushed
the
feat/display-video-wallpaper-thumbnail
branch
from
3a12fdf to
aa06497
Compare
|
TAG Bot New tag: 6.1.98 |
|
TAG Bot New tag: 6.1.99 |
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
2 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
When the wallpaper is a video file, resolve its thumbnail (from metadata.json, ffmpegthumbnailer cache, or async generation) and pass the thumbnail path to QML instead of the video path, so the monitor schematic can display dynamic wallpaper thumbnails correctly.
Changes
resolveVideoThumbnail()to DisplayWorker for video wallpaperisVideoFile()helper andWallpaperSourceTypeVideoconstantTest Plan
Fixes: https://pms.uniontech.com/bug-view-367369.html
Summary by Sourcery
Support using thumbnails for video wallpapers in the monitor schematic instead of video files, including async thumbnail generation and caching.
New Features:
Enhancements:
Build: