RTC: Support statistic for HTTP-API, HTTP-Callback and Security #2483
Conversation
|
Kk |
Codecov Report
@@ Coverage Diff @@
## develop #2483 +/- ##
===========================================
+ Coverage 42.66% 42.71% +0.05%
===========================================
Files 102 102
Lines 36039 36052 +13
===========================================
+ Hits 15376 15400 +24
+ Misses 20663 20652 -11 | Impacted Files | Coverage Δ | |' Translated to English while maintaining the markdown structure: '| Impacted Files | Coverage Δ | | Translated to English while maintaining the markdown structure: | trunk/src/app/srs_app_rtc_api.cpp | Translated to English while maintaining the markdown structure: | trunk/src/app/srs_app_security.cpp | Continue to review full report at Codecov.
Translated to English while maintaining the markdown structure: '>
|
add SrsRtcConnPlay and SrsRtcConnPublish |
trunk/src/app/srs_app_rtc_conn.cpp
Outdated
| // update the statistic when client discoveried. | ||
| SrsStatistic* stat = SrsStatistic::instance(); | ||
| if ((err = stat->on_client(cid_.c_str(), req_, session_, SrsRtcConnPlay)) != srs_success) { | ||
| srs_trace("webrtc: add client failed!"); |
There was a problem hiding this comment.
- Can this err be ignored?
- Even if it is ignored, srs_freep(err) should still be done to avoid memory leaks.
TRANS_BY_GPT3
There was a problem hiding this comment.
on_client always returns srs_success, which can be ignored.
However, for the sake of code consistency, I will still change it to wrap.
TRANS_BY_GPT3
| @@ -1628,7 +1628,9 @@ SrsResponse::~SrsResponse() | |||
| string srs_client_type_string(SrsRtmpConnType type) | |||
| { | |||
| switch (type) { | |||
| case SrsRtmpConnPlay: return "Play"; | |||
| case SrsRtmpConnPlay: | |||
There was a problem hiding this comment.
Is it better to distinguish between these two and return rtmp-play and rtc-play?
TRANS_BY_GPT3
There was a problem hiding this comment.
At present, this field is not being used, but it can still be distinguished.
TRANS_BY_GPT3
| @@ -486,6 +486,9 @@ class SrsRtcConnection : public ISrsResource, public ISrsDisposingHandler | |||
| public: | |||
| virtual const SrsContextId& get_id(); | |||
| virtual std::string desc(); | |||
| // Interface ISrsExpire. | |||
| public: | |||
| virtual void expire(); | |||
There was a problem hiding this comment.
This interface supports kickoff. We need to test if there will be any issues.
client->conn->expire();
srs_warn("kickoff client id=%s ok", client_id.c_str());
TRANS_BY_GPT3
There was a problem hiding this comment.
Yes, it is used for the kickoff feature. I have tested it locally and it passed.
TRANS_BY_GPT3
trunk/src/app/srs_app_rtc_api.cpp
Outdated
| srs_parse_rtmp_url(streamurl, tcUrl, stream_name); | ||
|
|
||
| int port; | ||
| string schema, host, vhost, param; |
There was a problem hiding this comment.
There are too many scattered local variables, which can easily lead to misuse.
Can we move the declaration of SrsRtcUserConfig ruc; forward and directly use its variable, for example:
SrsRtcUserConfig ruc;
if (true) {
srs_parse_rtmp_url(streamurl, tcUrl, ruc.stream);
TRANS_BY_GPT3
There was a problem hiding this comment.
Okay, I'll make some changes.
TRANS_BY_GPT3
winlinvip
changed the title
winlinvip
changed the title
… v4.0.144 * commit message for your changes. Lines starting * Update srs_app_rtc_api.cpp * add SrsRtcConnPlay and SrsRtcConnPublish, in enum SrsRtmpConnType * Update srs_rtmp_stack.cpp * Update srs_app_rtc_conn.cpp * Update srs_app_rtc_api.cpp * update utest * Update srs_utest_app.cpp
No description provided.