-
-
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
rtmp2rtc: Support HEVC #4289
base: develop
Are you sure you want to change the base?
rtmp2rtc: Support HEVC #4289
Conversation
@@ -254,8 +258,10 @@ enum SrsRtspPacketPayloadType | |||
SrsRtspPacketPayloadTypeRaw, | |||
SrsRtspPacketPayloadTypeFUA2, | |||
SrsRtspPacketPayloadTypeFUA, | |||
SrsRtspPacketPayloadTypeFUAHevc, |
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.
Is the *Rtsp*
part of the removed RTSP related code? Is it should be renamed to SrsRtpPacketPayloadType
?
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.
maybe it is a typo introduced by @winlinvip
return false; | ||
} | ||
|
||
bool srs_sdp_has_h265_profile(const SrsSdp& sdp, const string& profile) |
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.
this is a so general method, the profile-id
defined in h.265 standard, but just main profile(1), main 10 profile(2), and main still picture profile(3) are commonly used.
why not add more specific wrapper method. e.g. srs_sdp_has_h265_main_profile(const SrsSdp& sdp)
,
srs_sdp_has_h265_main_10_profile(sdp)
and srs_sdp_has_h265_main_still_picture_profile(sdp)
.
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.
To align with H264, and we can improve it in another PR.
fc9fa0a
to
1a224c7
Compare
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.
About the SDP parameter and codec name case sensitive issues.
format_specific_param << "level-id=" << h265_param_.level_id; | ||
} | ||
if (!h265_param_.profile_id.empty()) { | ||
format_specific_param << ";profile-id=" << h265_param_.profile_id; |
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.
if the level_id
is empty, the format_specific_param will output with
;` ahead.
Is this case possible?
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.
Yes, H264 has the same issue. However, calling set_h265_param_desc
ensures that level_id
is not null.
if (kv.size() != 2) { | ||
return srs_error_new(ERROR_RTC_SDP_DECODE, "invalid h265 param=%s", attribute.c_str()); | ||
} | ||
if (kv[0] == "level-id") { |
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 didn't read the SDP spec, but only asked the AI whether the SDP parameters case sensitive?
It said the SDP parameters are generally case-insensitive. But the best practice is to use consistent casing, so it has some conventions there, e.g.
a=rtpmap:96 H264/90000
a=fmtp:96 profile-level-id=42e01f; packetization-mode=1
H264
andh264
are equivalent. (srs already handle this, I do think so)profile-level-id
andPROFILE-LEVEL-ID
are equivalent. (srs not handle this case here at least).- the whitespace between parameters. (e.g. profile-level-id=42e01f;
_
packetization-mode=1).
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.
It appears the protocol does not explicitly state whether spaces are allowed, but based on experience, they are not used.
|
||
// TODO: FIXME: pick up a profile for HEVC. | ||
// @see https://www.rfc-editor.org/rfc/rfc7798#section-7.2.1 | ||
track_descs = source->get_track_desc("video", "H265"); |
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.
About this method get_track_desc
, I would suggest for refactor later.
- there are only 3 kinds of track types
audio
,video
,data
, the SRS only has 2 of them,audio/video
. Aenum
is much better here. - Inside
get_track_desc
, there is a hide precondition of themedia_name
, the audio codec name is lower casing, while the video codec name is upper case. This method did the force case transform for the stored media, but forget do same transform for themedia_name
parameter.
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.
That's true. I've also been affected by this when doing case conversions. We can solve this problem with another PR
Currently Safari supports rtc2rtc. We should test the performance in Safari browser. I haven't run rtmp2rtc in Safari yet. |
Case 1:
Case 2:
So I think the rtc to rtmp bridge not consider this case well. Case 3:
|
I think this PR only supports hevc by rtmp2rtc, hevc by rtc2rtc can be fixed in the new PR. |
@suzp1984 rtc to rtmp is NOT support by now, I will start working on this after the PR is merged. |
We should test WebRTC 265 on Safari 18.0 and above.
|
trunk/src/app/srs_app_rtc_source.cpp
Outdated
@@ -1083,6 +1084,10 @@ srs_error_t SrsRtcRtpBuilder::on_video(SrsSharedPtrMessage* msg) | |||
if (has_idr) { | |||
SrsUniquePtr<SrsRtpPacket> pkt(new SrsRtpPacket()); | |||
|
|||
if ((err = bridge_->update_codec(format->vcodec->id)) != 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.
Each keyframe invokes the source_->get_track_desc
method, which could be moved to the initialization section.
TRANS_BY_GPT4
cd7598a
to
8953dbf
Compare
1. Usage
Launch Chrome with H.265 enabled:
Launch SRS with
rtmp2rtc.conf
Push H.265 with RTMP
Play with WebRTC
2. Parameter Combinations for SDP
sendrecv offer
sendonly offer
recvonly offer
3. How to test if H.265 is working
https://webrtc.github.io/samples/src/content/peerconnection/change-codecs/