feat: add dvd+rw-format support for DVD RW erase#347
Merged
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs 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 |
|
Warning
|
Reviewer's GuideIntroduce a dedicated DVDRwFormatEngine that uses dvd+rw-format for DVD±RW erase, wire it into DOpticalDiscManager with media-type detection and progress parsing, add dvdrwformatengine implementation, and declare dvd+rw-tools as a package dependency while keeping xorriso as a fallback path. Sequence diagram for DVD±RW erase flow with DVDRwFormatEngine and xorriso fallbacksequenceDiagram
actor User
participant DOpticalDiscManager
participant DOpticalDiscInfo
participant DVDRwFormatEngine
participant DXorrisoEngine
User->>DOpticalDiscManager: erase()
DOpticalDiscManager->>DOpticalDiscInfo: createOpticalInfo(curDev)
DOpticalDiscManager->>DOpticalDiscInfo: mediaType()
DOpticalDiscManager->>DVDRwFormatEngine: doErase(curDev, mediaType)
alt isDvdRw and dvd+rw-format found
DVDRwFormatEngine->>DVDRwFormatEngine: doErase(dev, type)
DVDRwFormatEngine-->>DOpticalDiscManager: jobStatusChanged(JobStatus, progress)
DOpticalDiscManager-->>User: jobStatusChanged(JobStatus, progress, {}, {})
else dvd+rw-format not found or not DVD±RW
DOpticalDiscManager->>DXorrisoEngine: doErase(curDev)
DXorrisoEngine-->>DOpticalDiscManager: jobStatusChanged(JobStatus, progress)
DOpticalDiscManager-->>User: jobStatusChanged(JobStatus, progress, {}, {})
end
Sequence diagram for dvd+rw-format progress parsing in DVDRwFormatEnginesequenceDiagram
participant DVDRwFormatEngine
participant QProcess
DVDRwFormatEngine->>QProcess: start(binary, args)
QProcess-->>DVDRwFormatEngine: readyReadStandardOutput()
DVDRwFormatEngine->>QProcess: readAllStandardOutput()
DVDRwFormatEngine->>DVDRwFormatEngine: parseProgress(buffer)
DVDRwFormatEngine-->>DVDRwFormatEngine: jobStatusChanged(JobStatus::kRunning, progress)
QProcess-->>DVDRwFormatEngine: finished(exitCode)
alt exitCode == 0
DVDRwFormatEngine-->>DVDRwFormatEngine: jobStatusChanged(JobStatus::kFinished, 100)
else exitCode != 0
DVDRwFormatEngine-->>DVDRwFormatEngine: jobStatusChanged(JobStatus::kFailed, -1)
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 1 issue, and left some high level feedback:
- You already guard on dvd+rw-format availability in DOpticalDiscManager::erase; consider removing the second QStandardPaths::findExecutable call from DVDRwFormatEngine::doErase to avoid redundant lookups and simplify the flow.
- In DVDRwFormatEngine::doErase, you use a nested QEventLoop with a QProcess; if this is called from UI or other event-driven contexts, it may be safer to run the process fully asynchronously (signals only) to avoid potential re-entrancy or blocking issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You already guard on dvd+rw-format availability in DOpticalDiscManager::erase; consider removing the second QStandardPaths::findExecutable call from DVDRwFormatEngine::doErase to avoid redundant lookups and simplify the flow.
- In DVDRwFormatEngine::doErase, you use a nested QEventLoop with a QProcess; if this is called from UI or other event-driven contexts, it may be safer to run the process fully asynchronously (signals only) to avoid potential re-entrancy or blocking issues.
## Individual Comments
### Comment 1
<location path="src/dfm-burn/dfm-burn-lib/private/dvdrwformatengine.cpp" line_range="98-101" />
<code_context>
+ // 0.0%\b\b\b\b0.1%\b|\b/\b\b0.2% ...
+ // 也有部分行带换行符（子进程的诊断输出、父进程结束时的 \n）。
+ // 在整个缓冲区中搜索最后一个 XX.X% 模式即可获取当前进度。
+ static const QRegularExpression re("([0-9]+\\.[0-9]+)%");
+ auto matches = re.globalMatch(QString::fromLocal8Bit(data));
+ QString lastPct;
+ while (matches.hasNext())
+ lastPct = matches.next().captured(1);
+ if (!lastPct.isEmpty()) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Progress parsing is tightly coupled to a fractional XX.X% format and may miss integer-only percentages.
The regex currently only matches values like `10.0%` (`[0-9]+\.[0-9]+%`). If dvd+rw-format outputs `10%` (no decimal) or slightly changes its format, progress will stop updating without errors. Consider supporting both integer and fractional percentages, e.g. `([0-9]+(?:\.[0-9]+)?)%`, and tuning the pattern (anchoring/loosening) to match the documented output formats reliably.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 33,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
|
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 33,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
|
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 33,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
|
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 33,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
|
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 33,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
|
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 33,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
|
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 33,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
|
1. Added new DVDRwFormatEngine class to handle DVD RW erasure using dvd+rw-tools 2. Modified control files to add dvd+rw-tools as a dependency 3. Updated DOpticalDiscManager to detect DVD±RW media and use new engine when available 4. Implemented progress parsing for dvd+rw-format's special output format 5. Added fallback to xorriso when dvd+rw-format is not available Log: Added support for DVD±RW erasure using dvd+rw-format tool Influence: 1. Test DVD+RW and DVD-RW erasure with dvd+rw-tools installed 2. Verify fallback to xorriso when dvd+rw-tools is missing 3. Check progress reporting during erase operations 4. Test with different DVD RW media types 5. Verify no regression in CD/DVD non-RW erase functionality feat: 添加对DVD RW擦除的dvd+rw-format支持 1. 新增DVDRwFormatEngine类用于通过dvd+rw-tools处理DVD RW擦除 2. 修改控制文件添加dvd+rw-tools作为依赖项 3. 更新DOpticalDiscManager以检测DVD±RW介质并在可用时使用新引擎 4. 实现针对dvd+rw-format特殊输出格式的进度解析 5. 添加当dvd+rw-format不可用时回退到xorriso的处理 Log: 新增使用dvd+rw-format工具进行DVD±RW擦除的支持 Influence: 1. 测试dvd+rw-tools安装时的DVD+RW和DVD-RW擦除功能 2. 验证dvd+rw-tools缺失时回退到xorriso的功能 3. 检查擦除过程中的进度报告 4. 使用不同类型的DVD RW介质进行测试 5. 验证普通CD/DVD非RW擦除功能无回归
|
Warning
|
deepin pr auto review★ 总体评分：93分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 --- a/src/dfm-burn/dfm-burn-lib/private/dvdrwformatengine.h
+++ b/src/dfm-burn/dfm-burn-lib/private/dvdrwformatengine.h
@@ -18,9 +18,12 @@ class DVDRwFormatEngine : public QObject
Q_OBJECT
public:
explicit DVDRwFormatEngine(QObject *parent = nullptr);
- ~DVDRwFormatEngine() override;
+ ~DVDRwFormatEngine() override = default;
bool doErase(const QString &dev, MediaType type);
+private:
+ Q_DISABLE_COPY(DVDRwFormatEngine)
+
Q_SIGNALS:
void jobStatusChanged(JobStatus status, int progress);
--- a/src/dfm-burn/dfm-burn-lib/private/dvdrwformatengine.cpp
+++ b/src/dfm-burn/dfm-burn-lib/private/dvdrwformatengine.cpp
@@ -88,12 +88,14 @@ bool DVDRwFormatEngine::doErase(const QString &dev, MediaType type)
Q_EMIT jobStatusChanged(JobStatus::kRunning, 0);
- // dvd+rw-format 进度输出机制分析（源码 dvd+rw-format.cpp）：
- //
- // 1. fork() 后子进程 setsid() 执行 SCSI 操作，父进程安装 SIGALRM
- // 定时轮询共享内存中的 progress 值（0~65536），用退格符覆盖式
- // 输出百分比到 stderr。
- //
- // 2. -gui 标志的 gui_action 赋值在 fork 之后的子进程中（727 行），
- // 父进程的 gui_action 始终为 NULL（129 行），所以 -gui 对父进程
- // 的进度输出格式无影响——父进程始终用 \b 退格覆盖方式。
- //
- // 3. DVD+RW -force 路径有 3 个 wait_for_unit(progress) 调用
- // （FORMAT UNIT → STOP DE-ICING → CLOSE SESSION），
- // 每个 SCSI 操作的进度指示器独立从 0 开始，
- // 导致原始输出出现 3 个 0→100% 周期。
- // DVD-RW -blank 路径只有 1 个 wait_for_unit，单周期。
- //
- // 4. 进度数据通过 stderr 管道传输。QProcess::readyReadStandardOutput
- // 信号在 QEventLoop + MergedChannels 下可能不能及时触发（尤其在
- // DVD-RW 单阶段输出稀疏时）。改用 waitForReadyRead 主动轮询
- // 管道——其内部调用 processEvents 处理 Qt 信号，进度可实时传递。
+ // dvd+rw-format 进度输出机制分析（源码 dvd+rw-format.cpp）：
+ // 父进程用退格符覆盖式输出百分比到 stderr，DVD+RW -force 有 3 个
+ // SCSI 操作阶段（FORMAT UNIT → STOP DE-ICING → CLOSE SESSION），
+ // DVD-RW -blank 只有 1 个阶段。使用 waitForReadyRead 主动轮询管道，
+ // 信号通过 Qt::DirectConnection 同步传递给上层，无需 processEvents。
int totalPhases = (type == MediaType::kDVD_PLUS_RW) ? 3 : 1;
@@ -108,11 +110,19 @@ bool DVDRwFormatEngine::doErase(const QString &dev, MediaType type)
proc.start(binary, args);
if (!proc.waitForStarted()) {
qWarning() << "[dfm-burn] Failed to start dvd+rw-format";
Q_EMIT jobStatusChanged(JobStatus::kFailed, -1);
return false;
}
- // 主动轮询管道，不依赖 readyReadStandardOutput 信号。
- // waitForReadyRead 内部调用 processEvents，emit 的信号能被上层接收。
+ // 主动轮询管道，不依赖 readyReadStandardOutput 信号。
+ // 信号通过 Qt::DirectConnection 同步传递，无需事件循环处理。
const int pollInterval { 200 }; // ms
+ const qint64 timeoutMs { 30 * 60 * 1000 }; // 30 分钟超时
+ QElapsedTimer timer;
+ timer.start();
while (proc.state() == QProcess::Running) {
+ if (timer.hasExpired(timeoutMs)) {
+ qWarning() << "[dfm-burn] dvd+rw-format timed out after" << timeoutMs << "ms";
+ proc.kill();
+ proc.waitForFinished(5000);
+ Q_EMIT jobStatusChanged(JobStatus::kFailed, -1);
+ return false;
+ }
proc.waitForReadyRead(pollInterval);
if (proc.bytesAvailable() > 0) { |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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.
dvd+rw-tools
when available
format
Log: Added support for DVD±RW erasure using dvd+rw-format tool
Influence:
feat: 添加对DVD RW擦除的dvd+rw-format支持
Log: 新增使用dvd+rw-format工具进行DVD±RW擦除的支持
Influence:
Summary by Sourcery
Add support for erasing DVD±RW media via a dedicated dvd+rw-format engine with progress reporting and fallback to existing xorriso-based erase.
New Features:
Enhancements:
Build: