Skip to content

Conversation

@add-uos
Copy link
Contributor

@add-uos add-uos commented Sep 8, 2025

When adjusting the system time, the timestamp will change according to the system time change during the recording process, resulting in the problem of time jumping.

Log: adjust timestamp for audio stream.
Bug: https://pms.uniontech.com/bug-view-268369.html

When adjusting the system time, the timestamp will change according to
the system time change during the recording process, resulting in the
problem of time jumping.

Log: adjust timestamp for audio stream.
Bug: https://pms.uniontech.com/bug-view-268369.html
@deepin-ci-robot
Copy link

deepin pr auto review

我对这段代码审查如下:

  1. 代码逻辑:
  • 代码主要实现了音视频输入流的处理,包括麦克风音频、系统音频的捕获和混合
  • 将原来的av_gettime()函数替换为TimeUtils::getMonotonicTime(),这是一个好的改进,因为单调时钟更适合用于测量时间间隔
  1. 语法:
  • 代码语法正确,符合C++标准
  • 头文件包含正确,新增的TimeUtils.h包含位置合适
  1. 代码质量:
  • 代码结构清晰,功能划分合理
  • 注释清晰,特别是对时间戳计算的说明很有帮助
  • 统一使用TimeUtils::getMonotonicTime()替代av_gettime(),提高了代码的一致性
  1. 性能:
  • 使用单调时钟可以提高时间测量的准确性
  • 保持了原有的性能优化,如帧处理逻辑不变
  1. 安全性:
  • 时间戳计算使用了64位整数(int64_t),可以避免溢出
  • 线程安全方面保持了原有的设计,没有引入新的风险

改进建议:

  1. 错误处理:
  • 建议在TimeUtils::getMonotonicTime()调用处添加错误检查,确保时间获取成功
  • 例如:
int64_t currentTime = TimeUtils::getMonotonicTime();
if (currentTime == -1) {
    qWarning() << "Failed to get monotonic time";
    return;
}
  1. 常量定义:
  • 建议将时间相关的常量(如AV_TIME_BASE)定义为类的常成员变量,提高可维护性
  1. 代码注释:
  • 可以添加更多关于TimeUtils::getMonotonicTime()与av_gettime()区别的说明,帮助后续维护者理解选择原因
  1. 时间同步:
  • 考虑在音频混合时添加时间同步机制,确保不同音频流的时序一致性
  1. 日志记录:
  • 建议在关键时间点添加日志记录,便于调试和问题追踪

总体来说,这是一个良好的改进,将系统时间函数替换为更可靠的单调时钟函数。代码质量较高,保持了原有的功能和性能,同时提高了时间测量的准确性。建议在后续维护中关注上述改进点。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@add-uos
Copy link
Contributor Author

add-uos commented Sep 10, 2025

/merge

@deepin-bot deepin-bot bot merged commit 474742b into linuxdeepin:develop/wayland-recorde-accumulate Sep 10, 2025
7 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