From 28deb90728c06a35d8847d2aeda2fc1aee105c5e Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Wed, 30 May 2018 14:56:50 -0700 Subject: [PATCH] Reland "Start supporting H264 packetization mode 0." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a reland of 3409cfa378e75c0c08d900e0848147929249a62b Needed to change RtpVideoStreamReceiver to stop deregistering a payload type if two payload types refer to the same codec (which now happens, with the packetization mode 0/1 payload types). It's not clear why this was being done in the first place. Original change's description: > Start supporting H264 packetization mode 0. > > The work was already done to support it, but it wasn't being negotiated > in SDP. > > This means we'll now see 8 H264 payload types instead of 4; one for each > combination of BP/CBP profiles, packetization modes 0/1, and RTX/non-RTX. > This could be problematic in the future, since we're starting to run > out of dynamic payload types (using 25 of 32). > > Bug: chromium:600254 > Change-Id: Ief2340db77c796f12980445b547b87e939170fae > Reviewed-on: https://webrtc-review.googlesource.com/77264 > Commit-Queue: Taylor Brandstetter > Reviewed-by: Erik Språng > Reviewed-by: Magnus Jedvert > Cr-Commit-Position: refs/heads/master@{#23372} Bug: chromium:600254 Change-Id: Ice1acc05acd1543d9b46e918de2bba0694d86259 Reviewed-on: https://webrtc-review.googlesource.com/78399 Reviewed-by: Danil Chapovalov Reviewed-by: Erik Språng Reviewed-by: Niels Moller Commit-Queue: Taylor Brandstetter Cr-Commit-Position: refs/heads/master@{#23494} --- .../rtp_rtcp/include/rtp_payload_registry.h | 5 ---- .../rtp_rtcp/source/rtp_payload_registry.cc | 29 ------------------- modules/video_coding/codecs/h264/h264.cc | 24 ++++++++++----- video/rtp_video_stream_receiver.cc | 9 ------ video/rtp_video_stream_receiver.h | 1 - 5 files changed, 17 insertions(+), 51 deletions(-) diff --git a/modules/rtp_rtcp/include/rtp_payload_registry.h b/modules/rtp_rtcp/include/rtp_payload_registry.h index 928d158198..056e8fb543 100644 --- a/modules/rtp_rtcp/include/rtp_payload_registry.h +++ b/modules/rtp_rtcp/include/rtp_payload_registry.h @@ -41,11 +41,6 @@ class RTPPayloadRegistry { int32_t DeRegisterReceivePayload(int8_t payload_type); - int32_t ReceivePayloadType(const SdpAudioFormat& audio_format, - int8_t* payload_type) const; - int32_t ReceivePayloadType(const VideoCodec& video_codec, - int8_t* payload_type) const; - int GetPayloadTypeFrequency(uint8_t payload_type) const; rtc::Optional PayloadTypeToPayload( diff --git a/modules/rtp_rtcp/source/rtp_payload_registry.cc b/modules/rtp_rtcp/source/rtp_payload_registry.cc index 46a4ec17f1..357fb5a493 100644 --- a/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -226,35 +226,6 @@ void RTPPayloadRegistry::DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( } } -int32_t RTPPayloadRegistry::ReceivePayloadType( - const SdpAudioFormat& audio_format, - int8_t* payload_type) const { - assert(payload_type); - rtc::CritScope cs(&crit_sect_); - - for (const auto& it : payload_type_map_) { - if (PayloadIsCompatible(it.second, audio_format)) { - *payload_type = it.first; - return 0; - } - } - return -1; -} - -int32_t RTPPayloadRegistry::ReceivePayloadType(const VideoCodec& video_codec, - int8_t* payload_type) const { - assert(payload_type); - rtc::CritScope cs(&crit_sect_); - - for (const auto& it : payload_type_map_) { - if (PayloadIsCompatible(it.second, video_codec)) { - *payload_type = it.first; - return 0; - } - } - return -1; -} - int RTPPayloadRegistry::GetPayloadTypeFrequency( uint8_t payload_type) const { const auto payload = PayloadTypeToPayload(payload_type); diff --git a/modules/video_coding/codecs/h264/h264.cc b/modules/video_coding/codecs/h264/h264.cc index 7771b13a2f..4408fab82a 100644 --- a/modules/video_coding/codecs/h264/h264.cc +++ b/modules/video_coding/codecs/h264/h264.cc @@ -40,14 +40,17 @@ bool IsH264CodecSupported() { #endif } -SdpVideoFormat CreateH264Format(H264::Profile profile, H264::Level level) { +SdpVideoFormat CreateH264Format(H264::Profile profile, + H264::Level level, + const std::string& packetization_mode) { const rtc::Optional profile_string = H264::ProfileLevelIdToString(H264::ProfileLevelId(profile, level)); RTC_CHECK(profile_string); - return SdpVideoFormat(cricket::kH264CodecName, - {{cricket::kH264FmtpProfileLevelId, *profile_string}, - {cricket::kH264FmtpLevelAsymmetryAllowed, "1"}, - {cricket::kH264FmtpPacketizationMode, "1"}}); + return SdpVideoFormat( + cricket::kH264CodecName, + {{cricket::kH264FmtpProfileLevelId, *profile_string}, + {cricket::kH264FmtpLevelAsymmetryAllowed, "1"}, + {cricket::kH264FmtpPacketizationMode, packetization_mode}}); } } // namespace @@ -67,8 +70,15 @@ std::vector SupportedH264Codecs() { // decoder for that profile is required to be able to decode CBP. This means // we can encode and send CBP even though we negotiated a potentially // higher profile. See the H264 spec for more information. - return {CreateH264Format(H264::kProfileBaseline, H264::kLevel3_1), - CreateH264Format(H264::kProfileConstrainedBaseline, H264::kLevel3_1)}; + // + // We support both packetization modes 0 (mandatory) and 1 (optional, + // preferred). + return { + CreateH264Format(H264::kProfileBaseline, H264::kLevel3_1, "1"), + CreateH264Format(H264::kProfileBaseline, H264::kLevel3_1, "0"), + CreateH264Format(H264::kProfileConstrainedBaseline, H264::kLevel3_1, "1"), + CreateH264Format(H264::kProfileConstrainedBaseline, H264::kLevel3_1, + "0")}; } std::unique_ptr H264Encoder::Create( diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index f7a919ca81..589b0da805 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -171,15 +171,6 @@ bool RtpVideoStreamReceiver::AddReceiveCodec( const VideoCodec& video_codec, const std::map& codec_params) { pt_codec_params_.insert(make_pair(video_codec.plType, codec_params)); - return AddReceiveCodec(video_codec); -} - -bool RtpVideoStreamReceiver::AddReceiveCodec(const VideoCodec& video_codec) { - int8_t old_pltype = -1; - if (rtp_payload_registry_.ReceivePayloadType(video_codec, &old_pltype) != - -1) { - rtp_payload_registry_.DeRegisterReceivePayload(old_pltype); - } return rtp_payload_registry_.RegisterReceivePayload(video_codec) == 0; } diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 3c1e539521..4c8a4f2595 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -140,7 +140,6 @@ class RtpVideoStreamReceiver : public RtpData, void RemoveSecondarySink(const RtpPacketSinkInterface* sink); private: - bool AddReceiveCodec(const VideoCodec& video_codec); void ReceivePacket(const uint8_t* packet, size_t packet_length, const RTPHeader& header);