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

RTC: Supports Firefox and mobile access. Also fixes some memory leak bugs. #1998

Closed
wants to merge 7 commits into from

Conversation

duiniuluantanqin
Copy link
Member

@duiniuluantanqin duiniuluantanqin commented Oct 20, 2020

Fix the bug that the new version does not support Firefox and mobile access to WebRTC.

Reason
The payload type in the RTP header is inconsistent with the one in the SDP.

Solution
Save the payload type negotiated during SDP negotiation in the track. When calling send_packets, update this PT in each RTP header.

Test Results

The italic bold line indicates the newly added support (compared to version 4.0.39).

  1. PC端 (PC end)
    • Chrome v86.0.4240.75
    • Firefox v81.0.1
  2. Android
    • Chrome v85
  3. IOS
    • Safari v12

issue
Resolve the issues of the following 2 items.

Others

  1. Fix the memory leak issue in the SrsRtcConnection::add_player function.
  2. Fix the SDP exception issue in the generate_media_payload_type function when the first parameter is missing, such as a=fmtp 111 ;useinbandfec=1.
  3. Add two new audio parameters: stereo and maxplaybackrate.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Oct 23, 2020

👍 I took a quick look and didn't see any issues. I will find time to examine the merge in detail. Thank you, thank you.

TRANS_BY_GPT3

@winlinvip winlinvip closed this Oct 31, 2020
@winlinvip winlinvip deleted the branch ossrs:develop October 31, 2020 13:55
@duiniuluantanqin duiniuluantanqin deleted the feature/rtc branch November 2, 2020 01:28
@duiniuluantanqin duiniuluantanqin restored the feature/rtc branch November 2, 2020 01:28
@winlinvip winlinvip reopened this Nov 12, 2020
@winlinvip
Copy link
Member

winlinvip commented Nov 12, 2020

I tried it in detail, the PT in Answer must be in Offer.

For RTMP, currently SRS defaults to Chrome's PT (Opus/111, H264/102), while Firefox (Opus/109, H264/126) is different.

For RTC, SRS generates the Player's PT based on the Publisher's PT. So, if Chrome is used for streaming and playback, there are no issues. Similarly, if Firefox is used for both streaming and playback, there are no problems (confirmed). However, when Chrome is used for streaming and Firefox for playback, or vice versa, there may be situations where packets are received but cannot be played.

Therefore, the solution to this problem should be: The PT of the Answer should be generated based on the Offer, it should not be set as default or use the PT of the Publisher.

TRANS_BY_GPT3

@ghostsf
Copy link
Contributor

ghostsf commented Nov 12, 2020

I tried it in detail, and the PT in Answer must be in Offer.

For RTMP, currently SRS defaults to Chrome's PT (Opus/111, H264/102), while Firefox's PT (Opus/109, H264/126) is different.

For RTC, SRS generates the Player's PT based on the Publisher's PT. So, if Chrome is used for publishing and playback, there are no issues. Similarly, if Firefox is used for both publishing and playback, there are no issues (confirmed to be working fine). However, when there is a cross between Chrome publishing and Firefox playback, or Firefox publishing and Chrome playback, there may be a situation where packets are received but cannot be played.

Therefore, the solution to this problem should be: the PT of the Answer should be generated based on the Offer, it should not be set as default or use the PT of the Publisher.

Yes, please merge and process it quickly.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Nov 12, 2020

In addition, fmtp is also necessary, otherwise decoding will fail.

a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=1
a=fmtp:126 profile-level-id=42e01f;level-asymmetry-allowed=1;packetization-mode=1

Note: H.264 is necessary, but for Opus, further confirmation is required.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Nov 13, 2020

Firefox still has an issue where the Candidate cannot be configured as a domain name, it must be configured as an IP address.

You can change the domain name to an IP address in the configuration.

    rtc_server {
      enabled         on;
      listen          8000;
      # Firefox doesn't work with domain.
      #candidate       d.ossrs.net;
      candidate       39.107.238.185;
    }

TRANS_BY_GPT3

winlinvip added a commit that referenced this pull request Nov 13, 2020
@winlinvip winlinvip changed the base branch from feature/rtc to develop November 15, 2020 15:26
winlinvip added a commit that referenced this pull request Dec 1, 2020
winlinvip added a commit that referenced this pull request Dec 2, 2020
winlinvip added a commit that referenced this pull request Dec 2, 2020
Copy link
Member Author

@duiniuluantanqin duiniuluantanqin left a comment

Choose a reason for hiding this comment

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

Firefox still has an issue, Candidate cannot be configured as a domain name, it must be configured as an IP address:

You can change the domain name to an IP address in the configuration.

    rtc_server {
      enabled         on;
      listen          8000;
      # Firefox doesn't work with domain.
      #candidate       d.ossrs.net;
      candidate       39.107.238.185;
    }

The approach to solving this problem is what? Can someone give me some guidance? I will give it a try.

TRANS_BY_GPT3

@@ -1355,6 +1362,7 @@ SrsMediaPayloadType SrsAudioPayload::generate_media_payload_type()
format_specific_param << ";usedtx=1";
}
media_payload_type.format_specific_param_ = format_specific_param.str();
media_payload_type.format_specific_param_ = media_payload_type.format_specific_param_.substr(1);
Copy link
Member Author

@duiniuluantanqin duiniuluantanqin Nov 12, 2020

Choose a reason for hiding this comment

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

This line of code is used to remove the first semicolon. The purpose of having the first semicolon is to avoid formatting errors that may occur when the first parameter is missing.

TRANS_BY_GPT3

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

@winlinvip winlinvip changed the title RTC:支持Firefox和手机访问。同时修复一些内存泄漏BUG RTC: Supports Firefox and mobile access. Also fixes some memory leak bugs. Jul 29, 2023
@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.

3 participants