Support Simulcast To RTMP #2792
Conversation
(cherry picked from commit 98559793a446cafee1f06d1d5162937e78d26d21)
(cherry picked from commit b8a3796dfd3e38142014b2ec0667d5c2e3368e7d)
… rtmp. (cherry picked from commit 35e442a45def4beda2d8625114e731366935b98e)
(cherry picked from commit f7d3d392a998cced55fc82e169b21853dcd02fa1)
Codecov Report
@@ Coverage Diff @@
## feature/simulcast #2792 +/- ##
=====================================================
- Coverage 59.78% 59.62% -0.17%
=====================================================
Files 121 121
Lines 51394 51535 +141
=====================================================
Hits 30727 30727
- Misses 20667 20808 +141 | 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_source.hpp | Continue to review full report at Codecov.
Translated to English while maintaining the markdown structure: |
|
| return err; | ||
| } | ||
|
|
||
| srs_error_t do_bridger(SrsRtmpFromRtcBridger *&bridger, SrsRequest* r, std::vector<SrsRtcVideoRecvTrack*>& video_tracks) { |
There was a problem hiding this comment.
This should be classified as a member function of SrsRTCPublishStream.
TRANS_BY_GPT3
There was a problem hiding this comment.
Does this part of the code look like a Refactor improvement?
Please don't go off-topic, CR. This PR is to solve the issue of supporting Simulcast for RTMP, so unrelated modifications should not be made.
If you want to improve the code, please kindly create a separate PR. This way, it will be easier for everyone to reach a consensus.
Thank you!
TRANS_BY_GPT3
| @@ -1123,6 +1123,56 @@ SrsRtcPublishStream::~SrsRtcPublishStream() | |||
| stat->on_disconnect(cid_.c_str()); | |||
| } | |||
|
|
|||
| #ifdef SRS_FFMPEG_FIT | |||
| inline SrsRtmpFromRtcBridger* create_bridger(SrsLiveSource *rtmp) { | |||
There was a problem hiding this comment.
This writing style is a bit flashy. Currently, the hierarchy is not that complex. Can we directly create a new object at the place where this function is called?
TRANS_BY_GPT3
There was a problem hiding this comment.
This is to open a door for the development of future business using the factory pattern.
TRANS_BY_GPT3
There was a problem hiding this comment.
Do not design and use packaging excessively in advance. If the factory pattern is not currently needed, do not use it prematurely.
If the factory pattern is needed later, it is not too late to use it.
First of all, everyone should be able to understand why this change is made. We all need to be able to understand it, and this is the first principle. Understanding does not only mean being able to read the code, but also recognizing that this modification is reasonable and optimal.
TRANS_BY_GPT3
|
|
||
| extern srs_error_t do_bridger_for_single(SrsRtmpFromRtcBridger *&bridger, SrsRequest *r); | ||
|
|
||
| std::string update_stream(size_t i, const SrsRtcTrackDescription* track_desc, const SrsRequest* rr) { |
There was a problem hiding this comment.
You can add some comments here to indicate the change of stream name from simulcast rtc to rtmp.
TRANS_BY_GPT3
trunk/src/app/srs_app_rtc_source.cpp
Outdated
| SrsRtmpFromRtcBridger* bridger; | ||
|
|
||
| srs_error_t create_bridger(size_t i, SrsRequest* rr) { | ||
| assert(!bridger); |
There was a problem hiding this comment.
Should use srs_assert
TRANS_BY_GPT3
trunk/src/app/srs_app_rtc_source.cpp
Outdated
| public: | ||
| SimulcastBridgerAdapter(std::vector<SrsRtcVideoRecvTrack*>& video_tracks): SrsRtmpFromRtcBridger(nullptr) { | ||
| for (auto& track: video_tracks) { | ||
| contexts_.emplace_back(BridgerContext{track->track_desc_, nullptr, nullptr}); |
There was a problem hiding this comment.
C++11 is not recommended for temporary use
TRANS_BY_GPT3
| return err; | ||
| } | ||
|
|
||
| srs_error_t on_rtp(SrsRtpPacket *pkt) override { |
There was a problem hiding this comment.
This function seems to be duplicated
TRANS_BY_GPT3
There was a problem hiding this comment.
No, this is necessary.
TRANS_BY_GPT3
There was a problem hiding this comment.
SrsRtmpFromRtcBridger::on_rtp
srs_error_t on_rtp(SrsRtpPacket *pkt) override
Is it really unnecessary?
TRANS_BY_GPT3
trunk/src/app/srs_app_source.cpp
Outdated
| @@ -1669,6 +1670,7 @@ srs_error_t SrsMetaCache::update_data(SrsMessageHeader* header, SrsOnMetaDataPac | |||
|
|
|||
| srs_error_t SrsMetaCache::update_ash(SrsSharedPtrMessage* msg) | |||
| { | |||
| assert(msg->payload); | |||
There was a problem hiding this comment.
If you want to use assert, please use srs_assert.
TRANS_BY_GPT3
johzzy
force-pushed
the
johnny/feature/simulcast
branch
4 times, most recently
from
e405359
to
edaae38
Compare
| contexts_.emplace_back(BridgerContext{track->track_desc_, NULL, NULL}); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The entire function below is also repetitive code, only the implementation of dispatch_audio is necessary.
TRANS_BY_GPT3
trunk/src/app/srs_app_rtc_source.cpp
Outdated
| std::map<uint32_t, SrsRtmpFromRtcBridger*> bridgers_; | ||
| std::vector<SrsLiveSource*> sources_; | ||
|
|
||
| SrsCommonMessage copy; |
There was a problem hiding this comment.
This member variable is not needed, it can be directly defined in the function.
TRANS_BY_GPT3
| } | ||
| }; | ||
|
|
||
| class SimulcastBridgerAdapter: public SrsRtmpFromRtcBridger { |
There was a problem hiding this comment.
In addition, it is best to define classes in hpp files to maintain consistent code style.
TRANS_BY_GPT3
| break; | ||
| } | ||
|
|
||
| // restore |
There was a problem hiding this comment.
C++11 functions, not recommended, replace them.
TRANS_BY_GPT3
johzzy
force-pushed
the
johnny/feature/simulcast
branch
from
da9010c
to
02d0e88
Compare
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgerAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgeAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgeAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgeAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
* add SimulcastBridgeAdapter, support simulcast to rtmp. * shared rtmp packet for dispatch_audio.
Rule: One simulcast(low,mid,high) stream to Multiple rtmp streams ( rtmp://xxx_{low,mid,high} )
Related Link: #2420, #2659