Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calculate the correct payload_size which pure padding data, in the process of rtc2rtmp, make Chrome happy #2461

Merged
merged 3 commits into from
Jul 8, 2021

Conversation

duiniuluantanqin
Copy link
Member

@duiniuluantanqin duiniuluantanqin commented Jul 6, 2021

Phenomenon: When using rtc2rtmp and enabling twcc (enabled by default), there is an error when streaming with getDisplayMedia and playing flv. However, when twcc is disabled, the playback is normal.

Reason:

  1. Enabling twcc:
    1.1. If the actual encoding bandwidth is lower than the predicted bandwidth, padding will be sent actively.
    1.2. Through packet analysis, it is found that some packets are pure padding, meaning the payload_size of the effective payload is 0.
    Due to a bug in the code, the flv packets will be filled with many 0x00. As shown in the figure below.
    1.3. Firefox or ffplay will display an error when encountering such empty packets, but they choose to ignore and continue playing. However, Chrome has strict validation and directly reports an error, which causes the flv file to be unplayable.
  2. When twcc is disabled, there will be no padding packets, so the playback can proceed normally.

Solution:

  1. Calculate the correct payload_size. See the code for details.

Reference article: RTP padding in WebRTC m79

微信图片_20210706182243


TRANS_BY_GPT3

@duiniuluantanqin duiniuluantanqin changed the title discard empty nalu in the process of rtc2rtmp, make Chrome happy discard empty packet which pure padding , in the process of rtc2rtmp, make Chrome happy Jul 6, 2021
@duiniuluantanqin duiniuluantanqin changed the title discard empty packet which pure padding , in the process of rtc2rtmp, make Chrome happy discard empty packet which pure padding data, in the process of rtc2rtmp, make Chrome happy Jul 6, 2021
@duiniuluantanqin duiniuluantanqin changed the title discard empty packet which pure padding data, in the process of rtc2rtmp, make Chrome happy Calculate the correct payload_size which pure padding data, in the process of rtc2rtmp, make Chrome happy Jul 6, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2021

Codecov Report

Merging #2461 (5d765ba) into develop (13d015b) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2461      +/-   ##
===========================================
- Coverage    42.67%   42.67%   -0.01%     
===========================================
  Files          102      102              
  Lines        36028    36034       +6     
===========================================
+ Hits         15376    15377       +1     
- Misses       20652    20657       +5     

| Impacted Files | Coverage Δ | |'

Translated to English while maintaining the markdown structure:

'| Impacted Files | Coverage Δ | |
|---|---|---|
| trunk/src/app/srs_app_rtc_source.cpp | 7.94% <0.00%> (-0.04%) | ⬇️ |
| trunk/src/protocol/srs_service_utility.cpp | 72.97% <0.00%> (+0.54%) | ⬆️ |


Continue to review full report at Codecov.

Legend - Click here to learn more
Translated to English while maintaining the markdown structure:

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

Powered by Codecov. Last update 13d015b...5d765ba. Read the comment docs.

TRANS_BY_GPT3

@duiniuluantanqin duiniuluantanqin changed the title Calculate the correct payload_size which pure padding data, in the process of rtc2rtmp, make Chrome happy Calculate the correct payload_size which pure padding data, in the process of rtc2rtmp, make Chrome happy Jul 6, 2021
@duiniuluantanqin duiniuluantanqin force-pushed the fix-bug-issue-2403 branch 2 times, most recently from 325563b to 3486c90 Compare July 6, 2021 08:22
nb_payload += 4 + raw_payload->nn_payload;
continue;
}
}

if (5 == nb_payload) {
Copy link
Member

@winlinvip winlinvip Jul 7, 2021

Choose a reason for hiding this comment

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

Why is it 5? The magic number needs to have a comment.

TRANS_BY_GPT3

Copy link
Member Author

@duiniuluantanqin duiniuluantanqin Jul 7, 2021

Choose a reason for hiding this comment

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

aad19c1
I made some adjustments, and now this 5 can be more clear.

TRANS_BY_GPT3

make clear for magic number
@xialixin
Copy link
Contributor

xialixin commented Jul 7, 2021

review, completed

TRANS_BY_GPT3

@winlinvip winlinvip merged commit 7eee9aa into ossrs:develop Jul 8, 2021
winlinvip pushed a commit that referenced this pull request Jul 8, 2021
Calculate the correct payload_size which pure padding data, in the process of rtc2rtmp, make Chrome happy (#2461)

* Calculate the correct payload_size which pure padding data, in the process of rtc2rtmp, make Chrome happy

* make clear for magic number

make clear for magic number

* Update srs_app_rtc_source.cpp
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TransByAI Translated by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When using rtc2rtmp, an error occurs when streaming with getDisplayMedia and playing flv.
4 participants