Skip to content

fix(player): reset m_isChangePlayFile in play() for same-file scenario#398

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
Resurgamz:feature/fix-BUG365835
Jul 7, 2026
Merged

fix(player): reset m_isChangePlayFile in play() for same-file scenario#398
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
Resurgamz:feature/fix-BUG365835

Conversation

@Resurgamz

@Resurgamz Resurgamz commented Jul 7, 2026

Copy link
Copy Markdown

Fix by directly resetting m_isChangePlayFile in VlcPalyer::play() when MPV is already in Playing state after playByName(), rather than relying on onGetState() which is never triggered for the same file.

切换到同一文件的语音(如复制的语音笔记)时,MPV 不重新进入 Playing
状态(已经在 Playing),导致 onGetState 中 Playing && m_isChangePlayFile 分支不被触发,m_isChangePlayFile 保持 true。播放结束时 Idle && !m_isChangePlayFile 为 false,playEnd 不发出,进度条不消失。

在 VlcPalyer::play() 的 playByName 之后,若 MPV 已处于 Playing 状态, 直接重置 m_isChangePlayFile 标志,不依赖 onGetState 触发。

Log: 修复复制语音切换播放后进度条不消失的问题
Bug: https://pms.uniontech.com/bug-view-365835.html
Influence: 修复后切换播放同一语音文件时进度条正常更新且播放结束后正常消失,不影响不同语音文件的正常播放。

@sourcery-ai

sourcery-ai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Resets the m_isChangePlayFile flag directly in VlcPalyer::play() after playByName() when MPV is already in Playing state for same-file playback, ensuring playEnd is emitted and the progress bar clears correctly.

Sequence diagram for same-file playback flag reset in VlcPalyer::play

sequenceDiagram
    actor User
    participant UI as VoiceUI
    participant VlcPalyer
    participant PlayerEngine

    User ->> UI: select copied_voice_note
    UI ->> VlcPalyer: play()
    VlcPalyer ->> PlayerEngine: addPlayFile(videoUrl)
    VlcPalyer ->> PlayerEngine: playByName(videoUrl)
    alt same_file_already_playing
        PlayerEngine ->> VlcPalyer: state() == Playing
        VlcPalyer ->> VlcPalyer: m_isChangePlayFile = false
    end

    PlayerEngine ->> PlayerEngine: playback finishes (state Idle)
    PlayerEngine ->> VlcPalyer: onGetState(Idle)
    VlcPalyer ->> UI: playEnd (progress bar cleared)
Loading

File-Level Changes

Change Details Files
Ensure same-file playback correctly emits playEnd by resetting m_isChangePlayFile when the player is already in Playing state after playByName().
  • Add a post-playByName check for m_isChangePlayFile combined with the player core state.
  • Reset m_isChangePlayFile to false when the core state is Playing to handle same-file scenarios where onGetState is not triggered.
  • Document the behavior with inline comments explaining why the flag must be reset here instead of in onGetState.
src/common/vlcpalyer.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Consider centralizing the logic for resetting m_isChangePlayFile (currently split between onGetState and play()) into a shared helper to keep state-transition handling consistent and easier to reason about.
  • Calling player->state() immediately after playByName() assumes synchronous state updates; if the engine updates state asynchronously, this condition may be flaky—verify whether a more robust trigger (e.g., a callback or queued check) is needed for the same-file scenario.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider centralizing the logic for resetting m_isChangePlayFile (currently split between onGetState and play()) into a shared helper to keep state-transition handling consistent and easier to reason about.
- Calling player->state() immediately after playByName() assumes synchronous state updates; if the engine updates state asynchronously, this condition may be flaky—verify whether a more robust trigger (e.g., a callback or queued check) is needed for the same-file scenario.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Resurgamz
Resurgamz force-pushed the feature/fix-BUG365835 branch 5 times, most recently from 14f50dd to 5564734 Compare July 7, 2026 06:10
Fix by directly resetting m_isChangePlayFile in VlcPalyer::play()
when MPV is already in Playing state after playByName(), rather than
relying on onGetState() which is never triggered for the same file.

切换到同一文件的语音(如复制的语音笔记)时,MPV 不重新进入 Playing
状态(已经在 Playing),导致 onGetState 中 Playing && m_isChangePlayFile
分支不被触发,m_isChangePlayFile 保持 true。播放结束时 Idle && !m_isChangePlayFile
为 false,playEnd 不发出,进度条不消失。

在 VlcPalyer::play() 的 playByName 之后,若 MPV 已处于 Playing 状态,
直接重置 m_isChangePlayFile 标志,不依赖 onGetState 触发。

Log: 修复复制语音切换播放后进度条不消失的问题
Bug: https://pms.uniontech.com/bug-view-365835.html
Influence: 修复后切换播放同一语音文件时进度条正常更新且播放结束后正常消失,不影响不同语音文件的正常播放。
@Resurgamz
Resurgamz force-pushed the feature/fix-BUG365835 branch from 5564734 to 9066870 Compare July 7, 2026 07:23
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码精准修复了同一文件重复播放时状态标志未重置及播放位置异常的问题,逻辑严密且注释完善
逻辑正确无瑕疵,代码质量优秀,仅因版权年份跨度略显突兀扣除少量分数

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

在 VlcPalyer::play() 函数中,针对 MPV 引擎播放同一文件时不触发状态切换的边界情况,通过判断 m_isChangePlayFile 标志与当前播放状态,精准拦截并重置了状态,同时补充了 seekAbsolute(0) 调用,完全解决了 playEnd 信号丢失和无法从头播放的问题
建议:无

  • 2.代码质量(良好)✓

新增代码块包含详尽的多行注释,清晰阐述了 BUG 的根因(MPV 不重新进入 Playing 状态导致 onGetState 分支未触发)以及修复的必要性,极大提升了后续维护的可读性
建议:版权年份更新为 2019 - 2026 跨度较大,建议确认是否应更新为当前实际年份(如 2024 或 2025)

  • 3.代码性能(高效)✓

仅增加了一次布尔变量读取与一次枚举状态比较,在最坏情况下额外执行一次 seek 操作,对系统资源消耗和响应时间的影响可忽略不计
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码未引入任何外部输入,仅操作内部状态标志和调用播放器接口,不存在命令注入、内存越界或权限绕过等安全风险

  • 建议:保持当前的内部状态管理方式

■ 【改进建议代码示例】

// Copyright (C) 2019 - 2025 UnionTech Software Technology Co.,Ltd.
// SPDX-FileCopyrightText: 2023 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-or-later

// ... 其他代码 ...

void VlcPalyer::play()
{
    // ... 前置逻辑 ...
    if (!isPlayingSameFile) {
        qInfo() << "Play new audio: " << videoUrl;
        player->addPlayFile(videoUrl);
        player->playByName(videoUrl);
        // BUG修复:处理同一文件重复播放时 MPV 状态不切换导致的 m_isChangePlayFile 未重置及 playEnd 信号丢失问题
        // 同一文件场景:MPV 不重新进入 Playing 状态(已经在 Playing),
        // onGetState 中的 Playing && m_isChangePlayFile 分支不会被触发,
        // 需在此处直接重置标志,否则播放结束时 playEnd 不发出
        if (m_isChangePlayFile && player->state() == PlayerEngine::CoreState::Playing) {
            m_isChangePlayFile = false;
            // 同一文件 MPV 不会从头播放,需显式 seek 到起始位置
            player->seekAbsolute(0);
        }
    }
// ... 后续逻辑 ...
}

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: max-lvs, Resurgamz

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

@Resurgamz

Copy link
Copy Markdown
Author

/merge

@deepin-bot

deepin-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

This pr cannot be merged! (status: unstable)

@Resurgamz

Copy link
Copy Markdown
Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

This pr force merged! (status: unstable)

@deepin-bot
deepin-bot Bot merged commit c652ab3 into linuxdeepin:master Jul 7, 2026
21 of 23 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