Skip to content

RTC: Fix rtc to rtmp sync timestamp using sender report.#2470

Merged
winlinvip merged 12 commits intoossrs:4.0releasefrom
xiaozhihong:4.0release
Aug 16, 2021
Merged

RTC: Fix rtc to rtmp sync timestamp using sender report.#2470
winlinvip merged 12 commits intoossrs:4.0releasefrom
xiaozhihong:4.0release

Conversation

@xiaozhihong
Copy link
Copy Markdown
Collaborator

@xiaozhihong xiaozhihong commented Jul 13, 2021

Referenced this article: https://mp.weixin.qq.com/s/Tmr2Qk-h2BRa276_YFaBig
WebRTC uses SenderReport to synchronize the timestamps of audio and video during streaming. Without receiving SenderReport, it is impossible to determine the synchronization time. Therefore, it is not forwarded to RtmpFromRtcBridge to avoid further issues with rtmp/hls/flv/dvr caused by timestamp problems in the source stream.


TRANS_BY_GPT3

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 13, 2021

Codecov Report

Merging #2470 (57f81d7) into 4.0release (5e87627) will increase coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           4.0release    #2470      +/-   ##
==============================================
+ Coverage       59.03%   59.33%   +0.29%     
==============================================
  Files             122      122              
  Lines           51367    51556     +189     
==============================================
+ Hits            30326    30592     +266     
+ Misses          21041    20964      -77     

| Impacted Files | Coverage Δ | |'

Translated to English while maintaining the markdown structure:

'| Impacted Files | Coverage Δ | |
|---|---|---|
| trunk/src/app/srs_app_rtc_codec.cpp | 1.79% <ø> (+1.79%) | ⬆️ |
| trunk/src/app/srs_app_rtc_conn.cpp | 10.45% <ø> (+0.77%) | ⬆️ |
| trunk/src/app/srs_app_rtc_source.cpp | 12.39% <ø> (+4.48%) | ⬆️ |
| trunk/src/app/srs_app_rtc_source.hpp | 9.09% <ø> (+9.09%) | ⬆️ |
| trunk/src/kernel/srs_kernel_rtc_rtp.cpp | 18.29% <ø> (+0.75%) | ⬆️ |
| trunk/src/kernel/srs_kernel_rtc_rtp.hpp | 45.45% <ø> (+5.45%) | ⬆️ |'

Translated to English while maintaining the markdown structure:

'| trunk/src/app/srs_app_rtc_codec.cpp | 1.79% <ø> (+1.79%) | ⬆️ |
| trunk/src/app/srs_app_rtc_conn.cpp | 10.45% <ø> (+0.77%) | ⬆️ |
| trunk/src/app/srs_app_rtc_source.cpp | 12.39% <ø> (+4.48%) | ⬆️ |
| trunk/src/app/srs_app_rtc_source.hpp | 9.09% <ø> (+9.09%) | ⬆️ |
| trunk/src/kernel/srs_kernel_rtc_rtp.cpp | 18.29% <ø> (+0.75%) | ⬆️ |
| trunk/src/kernel/srs_kernel_rtc_rtp.hpp | 45.45% <ø> (+5.45%) | ⬆️ |
| trunk/src/utest/srs_utest_rtc.cpp | 99.34% <0.00%> (+0.20%) | ⬆️ |
| trunk/src/app/srs_app_conn.cpp | 46.52% <0.00%> (+0.25%) | ⬆️ |
| ... and 6 more | |


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data'

Translated to English while maintaining the markdown structure:

'> Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 5e87627...57f81d7. Read the comment docs.

TRANS_BY_GPT3

@winlinvip winlinvip force-pushed the 4.0release branch 9 times, most recently from bf2c3a1 to 5e87627 Compare August 16, 2021 01:15
@duiniuluantanqin duiniuluantanqin linked an issue Aug 16, 2021 that may be closed by this pull request
@winlinvip winlinvip changed the title rtc to rtmp sync timestamp using sender report. RTC: Fix rtc to rtmp sync timestamp using sender report. Aug 16, 2021
@winlinvip winlinvip merged commit ea8cff6 into ossrs:4.0release Aug 16, 2021
winlinvip added a commit to winlinvip/srs that referenced this pull request Aug 16, 2021
@xiaozhihong xiaozhihong linked an issue Nov 10, 2021 that may be closed by this pull request
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 29, 2023
winlinvip added a commit that referenced this pull request May 29, 2025
In the scenario of converting WebRTC to RTMP, this conversion will not
proceed until an SenderReport is received; for reference, see:
#2470.
Thus, if HTTP-FLV streaming is attempted before the SR is received, the
FLV Header will contain only audio, devoid of video content.
This error can be resolved by disabling `guess_has_av` in the
configuration file, since we can guarantee that both audio and video are
present in the test cases.

However, in the original regression tests, the
`TestRtcPublish_HttpFlvPlay` test case contains a bug:

https://github.com/ossrs/srs/blob/5a404c089baa93b906d2452ef47e2ba8a9e6211c/trunk/3rdparty/srs-bench/srs/rtc_test.go#L2421-L2424

The test would pass when `hasAudio` is true and `hasVideo` is false,
which is actually incorrect. Therefore, it has been revised so that the
test now only passes if both values are true.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: winlin <winlinvip@gmail.com>
winlinvip added a commit that referenced this pull request May 29, 2025
In the scenario of converting WebRTC to RTMP, this conversion will not
proceed until an SenderReport is received; for reference, see:
#2470.
Thus, if HTTP-FLV streaming is attempted before the SR is received, the
FLV Header will contain only audio, devoid of video content.
This error can be resolved by disabling `guess_has_av` in the
configuration file, since we can guarantee that both audio and video are
present in the test cases.

However, in the original regression tests, the
`TestRtcPublish_HttpFlvPlay` test case contains a bug:

https://github.com/ossrs/srs/blob/5a404c089baa93b906d2452ef47e2ba8a9e6211c/trunk/3rdparty/srs-bench/srs/rtc_test.go#L2421-L2424

The test would pass when `hasAudio` is true and `hasVideo` is false,
which is actually incorrect. Therefore, it has been revised so that the
test now only passes if both values are true.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: winlin <winlinvip@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

TransByAI Translated by AI/GPT.

4 participants