Skip to content

fix(sdlplayer): handle multi-channel open failure and harden play_cb#676

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master
May 27, 2026
Merged

fix(sdlplayer): handle multi-channel open failure and harden play_cb#676
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor

@dengzhongyuan365-dev dengzhongyuan365-dev commented May 27, 2026

When playing media with > 2 channels (e.g. 7-channel WAV/AC3), some
audio sinks reject the multi-channel SDL_AudioSpec, leaving the device
unopened and producing no audio or distorted output. libvlc_audio_setup_cb
previously bailed out as soon as SDL_OpenAudio returned -1, so any sink
that did not accept the source channel layout silently broke playback.

Retry SDL_OpenAudio with channels=2 before giving up so playback still
works on stereo-only outputs. Other small fixes / hardening on top of
that: resolve "SDL_Init" by its real name (it was being looked up as
"SDL_GetAudioStatus" and slipping through only because resolveSdlSymbol
silently returned NULL), call SDL_CloseAudio before reopening in
setup_cb to avoid leaking the previous device when VLC reconfigures
the format, log SDL_OpenAudio failures on resume() instead of swallowing
them, and lower SDL audio log priority from ERROR to WARN so useful
diagnostics are no longer filtered out.

Also harden libvlc_audio_play_cb against edge cases that could
previously corrupt the mix buffer or read past the source samples:
compute the buffer size with size_t to avoid int overflow at high
sample rates / large buffers, guard against zero rate / channels /
count, and split the channel-mismatch path into src>dst (truncate),
src==dst (memcpy), src<dst (pad with last channel) so we never read
past the source buffer.

将 SDL 播放器在多声道设备打开失败时的处理从直接报错改为自动降级
到立体声重试。原方案在 SDL_OpenAudio 拒绝多声道 spec 时 setup_cb
会立刻返回 -1,导致 7 声道 WAV/AC3 等文件在不支持多声道的输出端
完全没声音或破音。修复后即便 sink 只接受立体声,播放也能继续。
同时修正 SDL_Init 符号名误写为 SDL_GetAudioStatus 的 typo、setup
重新打开设备前先 CloseAudio 防止句柄泄漏、为 resume 路径下的
OpenAudio 失败补日志、SDL 日志等级 ERROR -> WARN 保留可观察性。
play_cb 入口加固:用 size_t 累乘 size 避免高采样率下 int 溢出,
零参数兜底,声道不匹配按 src>dst / == / < 三路分别处理,避免
越界读源 buffer。

Log: 修复多声道音频文件(如 7 声道 WAV/AC3)在仅支持立体声的音频输出设备上无声或破音的问题

PMS: BUG-362915

Influence: 修复后多声道源在不支持对应声道布局的输出设备上会自动降级到立体声继续播放;play_cb 在异常采样率/通道数下不再读越界,高采样率长 buffer 不再有 int 溢出风险。

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @dengzhongyuan365-dev, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@dengzhongyuan365-dev dengzhongyuan365-dev force-pushed the master branch 2 times, most recently from 7ed79f1 to 27931bb Compare May 27, 2026 06:44
When playing media with > 2 channels (e.g. 7-channel WAV/AC3), some
audio sinks reject the multi-channel SDL_AudioSpec, leaving the device
unopened and producing no audio or distorted output. libvlc_audio_setup_cb
previously bailed out as soon as SDL_OpenAudio returned -1, so any sink
that did not accept the source channel layout silently broke playback.

Retry SDL_OpenAudio with channels=2 before giving up so playback still
works on stereo-only outputs. Other small fixes / hardening on top of
that: resolve "SDL_Init" by its real name (it was being looked up as
"SDL_GetAudioStatus" and slipping through only because resolveSdlSymbol
silently returned NULL), call SDL_CloseAudio before reopening in
setup_cb to avoid leaking the previous device when VLC reconfigures
the format, log SDL_OpenAudio failures on resume() instead of swallowing
them, and lower SDL audio log priority from ERROR to WARN so useful
diagnostics are no longer filtered out.

Also harden libvlc_audio_play_cb against edge cases that could
previously corrupt the mix buffer or read past the source samples:
compute the buffer size with size_t to avoid int overflow at high
sample rates / large buffers, guard against zero rate / channels /
count, and split the channel-mismatch path into src>dst (truncate),
src==dst (memcpy), src<dst (pad with last channel) so we never read
past the source buffer.

将 SDL 播放器在多声道设备打开失败时的处理从直接报错改为自动降级
到立体声重试。原方案在 SDL_OpenAudio 拒绝多声道 spec 时 setup_cb
会立刻返回 -1,导致 7 声道 WAV/AC3 等文件在不支持多声道的输出端
完全没声音或破音。修复后即便 sink 只接受立体声,播放也能继续。
同时修正 SDL_Init 符号名误写为 SDL_GetAudioStatus 的 typo、setup
重新打开设备前先 CloseAudio 防止句柄泄漏、为 resume 路径下的
OpenAudio 失败补日志、SDL 日志等级 ERROR -> WARN 保留可观察性。
play_cb 入口加固:用 size_t 累乘 size 避免高采样率下 int 溢出,
零参数兜底,声道不匹配按 src>dst / == / < 三路分别处理,避免
越界读源 buffer。

Log: 修复多声道音频文件(如 7 声道 WAV/AC3)在仅支持立体声的音频输出设备上无声或破音的问题

PMS: BUG-362915

Influence: 修复后多声道源在不支持对应声道布局的输出设备上会自动降级到立体声继续播放;play_cb 在异常采样率/通道数下不再读越界,高采样率长 buffer 不再有 int 溢出风险。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff。本次修改主要涉及 .gitignore 的更新、版权信息的变更,以及对 sdlplayer.cpp 中 SDL 音频初始化、恢复、回调数据处理等逻辑的重要改进。

整体来看,这次修改显著提升了代码的健壮性和安全性,特别是修复了 VLA(变长数组)溢出风险、增加了 SDL 函数返回值检查、处理了多声道降级逻辑等。但仍有几处关键的安全和逻辑细节需要进一步优化。

以下是详细的审查意见:

一、 语法与逻辑

1. VLA(变长数组)栈溢出风险(严重)

+    const size_t size = static_cast<size_t>(count) * dstChannels * bytesPerSample;
     // ...
     char curSamples[size]; // 危险:VLA
  • 问题:虽然你将 size 的类型升级为了 size_t 避免了 int 溢出,但在 C++ 中使用 VLA(变长数组)是非标准的(仅 GNU 扩展支持)。更严重的是,如果 count 很大(例如高采样率音频的缓冲区),size 可能达到几 MB 甚至更大,直接分配在栈上会导致栈溢出,程序直接崩溃。
  • 建议:使用 std::vector<char>QByteArray 在堆上分配内存。
std::vector<char> curSamples(size);
// 后续 memcpy 的 curSamples 替换为 curSamples.data()

2. libvlc_audio_play_cb 中的除零风险

+    const unsigned bytesPerSample = sdlMediaPlayer->_rate / 8;
  • 问题:这里的命名和逻辑存在严重隐患。变量名叫 _rate(采样率),但代码用 _rate / 8 来表示“每样本字节数”。在 SDL 中,样本大小是由 SDL_AudioSpec.format(如 AUDIO_S16SYS)决定的,而不是采样率除以 8。如果 _rate 的值小于 8(虽然不太可能),这里会发生除零错误;更可能的是,如果 format 是 32位浮点数,这里算出来的字节数就是错的。
  • 建议:确认 _rate 在此处是否被错误复用。正确的每样本字节数应从音频格式推导,例如 SDL_AUDIO_BITSIZE(format) / 8。如果确认当前逻辑是项目历史遗留的特殊设计,必须增加防零检查:
if (sdlMediaPlayer->_rate == 0 || /* 其他除数 */) return;
const unsigned bytesPerSample = sdlMediaPlayer->_rate / 8; 

3. libvlc_audio_setup_cb 中的降级逻辑可能导致状态不一致

+        desiredAS.channels = 2;
+        if (OpenAudio(&desiredAS, &sdlMediaPlayer->obtainedAS) < 0) {
+            qCCritical(dmMusic) << "Failed to open audio device even with stereo";
+            return -1;
+        }
  • 问题:当打开音频设备失败时,你将 desiredAS.channels 强制设为 2 重试。但是,此时 sdlMediaPlayer->_channels 仍然是 VLC 传入的原始通道数(比如 6)。这会导致后续 libvlc_audio_play_cbsrcChannels(6)和 dstChannels(2)不一致,虽然你写了降级拷贝逻辑,但 VLC 传入的数据步长和 SDL 期望的步长差异需要非常谨慎地处理。
  • 建议:在降级成功后,最好同步更新 VLC 侧的通道数(如果允许),或者在日志中明确提示用户正在进行声道截断播放,以免用户疑惑音质变化。

二、 代码质量

1. 动态库符号解析缺少空指针检查(高风险)

+    SDL_CloseAudio_function CloseAudio = (SDL_CloseAudio_function)VlcDynamicInstance::VlcFunctionInstance()->resolveSdlSymbol("SDL_CloseAudio");
     // ...
+    CloseAudio();
  • 问题:项目通过动态加载(dlsym 等)获取 SDL 函数指针。但 CloseAudio 等新获取的函数指针在调用前没有进行 NULL 检查。如果 SDL 库版本不匹配或加载失败,调用空指针会导致段错误。
  • 建议:对所有通过 resolveSdlSymbol 获取的函数指针进行空指针校验:
if (!CloseAudio) {
    qCWarning(dmMusic) << "Failed to resolve SDL_CloseAudio";
} else {
    CloseAudio();
}

2. 冗余的 static_cast 转换

+    QByteArray ba(curSamples, static_cast<int>(size));
  • 问题QByteArray 的构造函数 QByteArray(const char *data, int size) 接受 int。你将 size_t size 强转为 int,如果 size 超过 INT_MAX(2GB),会发生截断甚至变为负数,导致数据丢失或构造异常。
  • 建议:如果采用了我前面建议的 std::vector,可以直接用迭代器构造;如果确认音频数据不可能超过 2GB,可以保留转型,但最好加上断言:Q_ASSERT(size <= INT_MAX);

3. .gitignore 文件末尾缺少换行符

+.codegraph/
\ No newline at end of file
  • 问题:POSIX 标准建议文本文件以换行符结尾,某些 Git 工具和 diff 算法在处理无尾部换行符的文件时可能会有警告。
  • 建议:在 .codegraph/ 后面加一个空行。

三、 代码性能

1. libvlc_audio_play_cb 中的逐帧内存拷贝(热点路径)

+        for (unsigned i = 0; i < count; ++i) {
+            for (unsigned j = 0; j < dstChannels; ++j) {
+                memcpy(curSamples + ..., static_cast<const char *>(samples) + ..., bytesPerSample);
+            }
+        }
  • 问题libvlc_audio_play_cb 是音频回调函数,属于极高频调用的热点路径。双层 for 循环内频繁调用 memcpy 进行极小数据块(通常 2 或 4 字节)的拷贝,函数调用开销和循环开销极大,可能导致音频卡顿。
  • 建议:如果 bytesPerSample 是编译期常量或可确定的值(如 2 或 4),可以通过指针强转(如 int16_t*)直接赋值,省去 memcpy 的开销;或者针对常见的声道转换(如 6ch 转 2ch)编写专门的 SIMD 或批量处理函数。目前逻辑保证了正确性,但在性能敏感场景需留意。

四、 代码安全

1. 源声道数越界读取风险

+        // 源声道少于目的:
+        for (unsigned i = 0; i < count; ++i) {
+            for (unsigned j = 0; j < dstChannels; ++j) {
+                const unsigned srcChan = (j < srcChannels) ? j : (srcChannels - 1);
+                memcpy(curSamples + (i * dstChannels + j) * bytesPerSample,
+                       static_cast<const char *>(samples) + (i * srcChannels + srcChan) * bytesPerSample,
+                       bytesPerSample);
+            }
+        }
  • 问题:这段兜底代码的逻辑是:如果目的声道多于源声道,多出来的声道用源数据的最后一个声道填充。但计算偏移量时:(i * srcChannels + srcChan) * bytesPerSample。当 i = count - 1srcChan = srcChannels - 1 时,读取的偏移量是 ((count-1)*srcChannels + srcChannels - 1) * bytesPerSample = (count*srcChannels - 1) * bytesPerSample
  • 结论:边界计算是正确的,没有越界读取风险。这部分逻辑安全可靠。

总结与修改建议代码片段

针对最严重的 VLA 栈溢出和函数指针空指针问题,建议对 libvlc_audio_play_cblibvlc_audio_setup_cb 进行如下微调:

// libvlc_audio_play_cb 修改建议
const size_t size = static_cast<size_t>(count) * dstChannels * bytesPerSample;
if (size == 0) return;

// 使用堆内存代替栈上的 VLA,防止大 buffer 导致栈溢出
std::vector<char> curSamples(size);

if (srcChannels > dstChannels) {
    // ... 原有逻辑,curSamples 替换为 curSamples.data() ...
} else if (srcChannels == dstChannels) {
    memcpy(curSamples.data(), samples, size);
} else {
    // ... 原有逻辑,curSamples 替换为 curSamples.data() ...
}

Q_ASSERT(size <= INT_MAX); // 确保不发生截断
QByteArray ba(curSamples.data(), static_cast<int>(size));

// libvlc_audio_setup_cb 修改建议
SDL_CloseAudio_function CloseAudio = (SDL_CloseAudio_function)VlcDynamicInstance::VlcFunctionInstance()->resolveSdlSymbol("SDL_CloseAudio");
if (CloseAudio) {
    CloseAudio(); // 确保非空再调用
} else {
    qCWarning(dmMusic) << "Failed to resolve SDL_CloseAudio, skipping close operation";
}

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, wyu71

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 27, 2026

This pr cannot be merged! (status: blocked)

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit b39bc12 into linuxdeepin:master May 27, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants