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: For #1657, support http hooks on_play/stop/publish/unpublish #2509

Merged
merged 4 commits into from
Aug 10, 2021

Conversation

duiniuluantanqin
Copy link
Member

No description provided.

@duiniuluantanqin
Copy link
Member Author

duiniuluantanqin commented Aug 5, 2021

Due to the special nature of WebRTC, signaling and data are two separate channels.
Currently, the request to play/stream is done through the signaling channel, while the handling of stopping is done through the data channel.
Therefore, on_play/publish is placed in the signaling interaction process, while on_stop/unpublish is placed in the data processing flow.

TRANS_BY_GPT3

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2021

Codecov Report

Merging #2509 (6a99ba5) into develop (06f10b1) will decrease coverage by 0.06%.
The diff coverage is 3.38%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2509      +/-   ##
===========================================
- Coverage    42.70%   42.64%   -0.07%     
===========================================
  Files          102      102              
  Lines        36058    36117      +59     
===========================================
+ Hits         15400    15401       +1     
- Misses       20658    20716      +58     

| Impacted Files | Coverage Δ | |'

Translated to English while maintaining the markdown structure:

'| Impacted Files | Coverage Δ | |
|---|---|---|
| trunk/src/app/srs_app_rtc_api.cpp | 0.00% <0.00%> (ø) | |'

Translated to English while maintaining the markdown structure:

| trunk/src/app/srs_app_rtc_api.cpp | 0.00% <0.00%> (ø) | |
| trunk/src/app/srs_app_rtc_conn.cpp | 9.74% <7.40%> (-0.04%) | ⬇️ |
| trunk/src/protocol/srs_service_utility.cpp | 72.43% <0.00%> (-0.55%) | ⬇️ |


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 06f10b1...6a99ba5. Read the comment docs.

TRANS_BY_GPT3

@xiaozhihong
Copy link
Collaborator

xiaozhihong commented Aug 7, 2021

Record an idea, usually there is a "start streaming" and a "stop streaming", but it seems that WebRTC doesn't pay much attention to this point. The player provided by SRS also does not provide an entry for "stop streaming". In the future, it may be considered to add a status button on the SRS playback page. In the WebRTC scenario, after "start streaming", it will check the streaming status. If the streaming is normal, the button will change to "stop streaming". Users can actively stop the WebRTC streaming through the http API (signaling).

TRANS_BY_GPT3

@@ -379,6 +379,10 @@ SrsRtcPlayStream::SrsRtcPlayStream(SrsRtcConnection* s, const SrsContextId& cid)

SrsRtcPlayStream::~SrsRtcPlayStream()
{
if (req_) {
http_hooks_on_stop();
Copy link
Member

@winlinvip winlinvip Aug 10, 2021

Choose a reason for hiding this comment

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

This is the issue that Guanghua mentioned before, regarding the problem of calling HTTP hooks in the destructor function.

publish' and 'play' must be called synchronously because they have authentication functionality.

unpublish' and 'stop' definitely do not require authentication, so it is possible to call them asynchronously.

Refer to ISrsAsyncCallTask.

Calling HTTP in the destructor can lead to coroutine switching, which carries a high risk.

TRANS_BY_GPT3

@winlinvip winlinvip merged commit 345b691 into ossrs:develop Aug 10, 2021
@winlinvip winlinvip changed the title support http hooks for rtc: on_play/stop/publish/unpublish RTC: For #1657, support http hooks n_play/stop/publish/unpublish Aug 10, 2021
winlinvip pushed a commit that referenced this pull request Aug 10, 2021
* support http hooks for rtc: on_play/stop/publish/unpublish

* Update srs_app_rtc_conn.cpp

* Update srs_app_rtc_conn.cpp
@duiniuluantanqin duiniuluantanqin linked an issue Aug 10, 2021 that may be closed by this pull request
@duiniuluantanqin duiniuluantanqin changed the title RTC: For #1657, support http hooks n_play/stop/publish/unpublish RTC: For #1657, support http hooks on_play/stop/publish/unpublish Sep 3, 2021
@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
5 participants