-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Support tansmux RTC to RTMP #2204
Conversation
This comment has been minimized.
This comment has been minimized.
audio_foramt rtp; | ||
req_keyframe 0; | ||
keyframe_interval_print 10; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always feel that in terms of this configuration, it would be better to place it at the vhost level. This would allow certain vhosts to enable RTC to RTMP conversion, instead of requiring all RTC to be converted to RTMP.
Something like this:
vhost __defaultVhost__ {
rtc {
enabled on;
}
rtc_to_rtmp {
enabled on;
}
}
If Opus to AAC conversion is not supported temporarily, then you can configure:
rtc_to_rtmp {
enabled on;
opus discard;
}
If you need to configure the PLI interval, you can configure:
rtc_to_rtmp {
enabled on;
pli 10;
}
TRANS_BY_GPT3
return DEFAULT; | ||
} | ||
|
||
conf = conf->get("opus_payload_type"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opus PT is not globally fixed, it is negotiated through SDP.
In other words, there is a possibility that clients can use different PT values.
This is an unreasonable choice to globally configure Opus's PT.
TRANS_BY_GPT3
return err; | ||
} | ||
|
||
srs_error_t SrsRtcRtmpMuxer::mux_opusflv(SrsRtpPacket2* pkt, char** flv, int* nb_flv) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opus is encapsulated in FLV, which is a non-standard protocol, and it is not very suitable to include this part in open source.
In the end, it is still better to consider encapsulating Opus into AAC and using standard protocols.
If the first step does not support Opus to AAC conversion, it can be configured to discard Opus audio.
TRANS_BY_GPT3
|
||
// generate rtmp url to connect to. | ||
std::string hostport = "127.0.0.1:1935"; | ||
std::string url = "rtmp://"+ hostport + req_->get_stream_url(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to RTMP being a local streaming, the push streaming address is configured here. However, there are a few issues:
- The port may not necessarily be 1935, and the relationship between the URL and output is defined quite vaguely.
- Since it is a network operation, there are multiple steps involved in RTMP push streaming, requiring a significant amount of round-trip time (RTT). Additionally, converting RTC push streaming to RTMP is triggered by RTC push streaming, making it difficult to ensure consistent overall state.
For example, if RTC push streaming is ongoing and RTMP also needs to push streaming, there may be a situation where RTMP starts pushing streaming while RTC is still converting to RTMP, as the conversion process requires several round-trip times (RTTs).
For example, if RTC push streaming is successful, but there is a failure in the complex logic of RTMP push streaming, how RTC handles it, whether to disconnect or ignore it.
If implementing something like SrsRtcFromRtmpBridger instead of using network streaming, the overall consistency of the state can be better guaranteed, and the overall logic is also simpler.
TRANS_BY_GPT3
|
||
SrsRtcConsumer* consumer = NULL; | ||
SrsAutoFree(SrsRtcConsumer, consumer); | ||
if ((err = source->create_consumer(consumer)) != srs_success) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole logic is quite convoluted. It starts a separate coroutine to fetch RTP packets, then converts them into RTMP frames, and finally pushes the stream to the local machine via a socket.
- A RTC Consumer is created to fetch RTP packets, but it can actually directly obtain RTP packets from the RTC Source/Stream. You can refer to SrsRtcFromRtmpBridger for more information.
- There are several coordination issues with multiple coroutines, such as how to handle stream pushing failures, what to do when the RTC Stream is disconnected, and how to handle network disconnections (even though it is local, there is still a need for this logic).
TRANS_BY_GPT3
ae95246
to
c5bea4f
Compare
Fixed by #2303 |
For #2200, support RTC to RTMP conversion.
RTMP to RTC conversion feature, overall, this PR seems too complex. It involves modifying nearly 2000 lines of code, which poses risks to state consistency. I need to consider a simpler and more direct solution.
TRANS_BY_GPT3