From 3cdfcd88a14449a9b116cb6149e1348d3a1e4cb2 Mon Sep 17 00:00:00 2001 From: stefan Date: Fri, 30 Sep 2016 09:06:36 -0700 Subject: [PATCH] Revert of Use sps and pps to determine decodability of H.264 frames. (patchset #4 id:60001 of https://codereview.webrtc.org/2341713002/ ) Reason for revert: Broke browser_tests, e.g., WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityH264 Original issue's description: > Use sps and pps to determine decodability of H.264 frames. > > NOPRESUBMIT=true > BUG=webrtc:6208 > R=philipel@webrtc.org > > Committed: https://crrev.com/17b02633666f2f5d7e78767ad5674c728d639c26 > Cr-Commit-Position: refs/heads/master@{#14458} TBR=philipel@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6208 Review-Url: https://codereview.webrtc.org/2381233004 Cr-Commit-Position: refs/heads/master@{#14460} --- .../rtp_rtcp/source/rtp_format_h264.cc | 21 +---- .../source/rtp_format_h264_unittest.cc | 12 ++- webrtc/modules/video_coding/decoding_state.cc | 75 +--------------- webrtc/modules/video_coding/decoding_state.h | 8 -- webrtc/modules/video_coding/frame_buffer.cc | 4 - webrtc/modules/video_coding/frame_buffer.h | 4 - .../video_coding/jitter_buffer_unittest.cc | 90 +------------------ webrtc/modules/video_coding/session_info.cc | 13 --- webrtc/modules/video_coding/session_info.h | 3 - 9 files changed, 17 insertions(+), 213 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc b/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc index d4729afba1..19467ca651 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc @@ -484,8 +484,6 @@ bool RtpDepacketizerH264::ProcessStapAOrSingleNalu( parsed_payload->type.Video.width = sps->width; parsed_payload->type.Video.height = sps->height; nalu.sps_id = sps->id; - } else { - LOG(LS_WARNING) << "Failed to parse SPS id from SPS slice."; } parsed_payload->frame_type = kVideoFrameKey; break; @@ -512,12 +510,8 @@ bool RtpDepacketizerH264::ProcessStapAOrSingleNalu( default: { rtc::Optional pps_id = PpsParser::ParsePpsIdFromSlice( &payload_data[start_offset], end_offset - start_offset); - if (pps_id) { + if (pps_id) nalu.pps_id = *pps_id; - } else { - LOG(LS_WARNING) << "Failed to parse PPS id from slice of type: " - << static_cast(nalu.type); - } break; } } @@ -553,13 +547,8 @@ bool RtpDepacketizerH264::ParseFuaNalu( length_ -= kNalHeaderSize; rtc::Optional pps_id = PpsParser::ParsePpsIdFromSlice( payload_data + 2 * kNalHeaderSize, length_ - kNalHeaderSize); - if (pps_id) { + if (pps_id) nalu.pps_id = *pps_id; - } else { - LOG(LS_WARNING) << "Failed to parse PPS from first fragment of FU-A NAL " - "unit with original type: " - << static_cast(nalu.type); - } uint8_t original_nal_header = fnri | original_nal_type; modified_buffer_.reset(new rtc::Buffer()); modified_buffer_->AppendData(payload_data + kNalHeaderSize, length_); @@ -581,10 +570,8 @@ bool RtpDepacketizerH264::ParseFuaNalu( RTPVideoHeaderH264* h264 = &parsed_payload->type.Video.codecHeader.H264; h264->packetization_type = kH264FuA; h264->nalu_type = original_nal_type; - if (first_fragment) { - h264->nalus[h264->nalus_length] = nalu; - h264->nalus_length = 1; - } + h264->nalus[h264->nalus_length] = nalu; + h264->nalus_length = 1; return true; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc index 1763c926ed..a5650f60e4 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc @@ -771,8 +771,10 @@ TEST_F(RtpDepacketizerH264Test, TestFuA) { const RTPVideoHeaderH264& h264 = payload.type.Video.codecHeader.H264; EXPECT_EQ(kH264FuA, h264.packetization_type); EXPECT_EQ(kIdr, h264.nalu_type); - // NALU info is only expected for the first FU-A packet. - EXPECT_EQ(0u, h264.nalus_length); + ASSERT_EQ(1u, h264.nalus_length); + EXPECT_EQ(static_cast(kIdr), h264.nalus[0].type); + EXPECT_EQ(-1, h264.nalus[0].sps_id); + EXPECT_EQ(-1, h264.nalus[0].pps_id); } payload = RtpDepacketizer::ParsedPayload(); @@ -785,8 +787,10 @@ TEST_F(RtpDepacketizerH264Test, TestFuA) { const RTPVideoHeaderH264& h264 = payload.type.Video.codecHeader.H264; EXPECT_EQ(kH264FuA, h264.packetization_type); EXPECT_EQ(kIdr, h264.nalu_type); - // NALU info is only expected for the first FU-A packet. - ASSERT_EQ(0u, h264.nalus_length); + ASSERT_EQ(1u, h264.nalus_length); + EXPECT_EQ(static_cast(kIdr), h264.nalus[0].type); + EXPECT_EQ(-1, h264.nalus[0].sps_id); + EXPECT_EQ(-1, h264.nalus[0].pps_id); } } diff --git a/webrtc/modules/video_coding/decoding_state.cc b/webrtc/modules/video_coding/decoding_state.cc index 530d167868..89be9b66c1 100644 --- a/webrtc/modules/video_coding/decoding_state.cc +++ b/webrtc/modules/video_coding/decoding_state.cc @@ -10,8 +10,6 @@ #include "webrtc/modules/video_coding/decoding_state.h" -#include "webrtc/base/logging.h" -#include "webrtc/common_video/h264/h264_common.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/video_coding/frame_buffer.h" #include "webrtc/modules/video_coding/jitter_buffer_common.h" @@ -42,8 +40,6 @@ void VCMDecodingState::Reset() { full_sync_ = true; in_initial_state_ = true; memset(frame_decoded_, 0, sizeof(frame_decoded_)); - received_sps_.clear(); - received_pps_.clear(); } uint32_t VCMDecodingState::time_stamp() const { @@ -78,24 +74,6 @@ void VCMDecodingState::SetState(const VCMFrameBuffer* frame) { temporal_id_ = frame->TemporalId(); tl0_pic_id_ = frame->Tl0PicId(); - for (const NaluInfo& nalu : frame->GetNaluInfos()) { - if (nalu.type == H264::NaluType::kPps) { - if (nalu.pps_id < 0) { - LOG(LS_WARNING) << "Received pps without pps id."; - } else if (nalu.sps_id < 0) { - LOG(LS_WARNING) << "Received pps without sps id."; - } else { - received_pps_[nalu.pps_id] = nalu.sps_id; - } - } else if (nalu.type == H264::NaluType::kSps) { - if (nalu.sps_id < 0) { - LOG(LS_WARNING) << "Received sps without sps id."; - } else { - received_sps_.insert(nalu.sps_id); - } - } - } - if (UsingFlexibleMode(frame)) { uint16_t frame_index = picture_id_ % kFrameDecodedLength; if (in_initial_state_) { @@ -128,8 +106,6 @@ void VCMDecodingState::CopyFrom(const VCMDecodingState& state) { in_initial_state_ = state.in_initial_state_; frame_decoded_cleared_to_ = state.frame_decoded_cleared_to_; memcpy(frame_decoded_, state.frame_decoded_, sizeof(frame_decoded_)); - received_sps_ = state.received_sps_; - received_pps_ = state.received_pps_; } bool VCMDecodingState::UpdateEmptyFrame(const VCMFrameBuffer* frame) { @@ -207,10 +183,8 @@ bool VCMDecodingState::ContinuousFrame(const VCMFrameBuffer* frame) const { // A key frame is always considered continuous as it doesn't refer to any // frames and therefore won't introduce any errors even if prior frames are // missing. - if (frame->FrameType() == kVideoFrameKey && - HaveSpsAndPps(frame->GetNaluInfos())) { + if (frame->FrameType() == kVideoFrameKey) return true; - } // When in the initial state we always require a key frame to start decoding. if (in_initial_state_) return false; @@ -231,8 +205,7 @@ bool VCMDecodingState::ContinuousFrame(const VCMFrameBuffer* frame) const { return ContinuousPictureId(frame->PictureId()); } } else { - return ContinuousSeqNum(static_cast(frame->GetLowSeqNum())) && - HaveSpsAndPps(frame->GetNaluInfos()); + return ContinuousSeqNum(static_cast(frame->GetLowSeqNum())); } } @@ -309,48 +282,4 @@ bool VCMDecodingState::AheadOfFramesDecodedClearedTo(uint16_t index) const { return diff > kFrameDecodedLength / 2; } -bool VCMDecodingState::HaveSpsAndPps(const std::vector& nalus) const { - std::set new_sps; - std::map new_pps; - for (const NaluInfo& nalu : nalus) { - switch (nalu.type) { - case H264::NaluType::kPps: - if (nalu.pps_id < 0) { - LOG(LS_WARNING) << "Received pps without pps id."; - } else if (nalu.sps_id < 0) { - LOG(LS_WARNING) << "Received pps without sps id."; - } else { - new_pps[nalu.pps_id] = nalu.sps_id; - } - break; - case H264::NaluType::kSps: - if (nalu.sps_id < 0) { - LOG(LS_WARNING) << "Received sps without sps id."; - } else { - new_sps.insert(nalu.sps_id); - } - break; - default: { - int sps_needed = -1; - auto pps_it = new_pps.find(nalu.pps_id); - if (pps_it != new_pps.end()) { - sps_needed = pps_it->second; - } else { - auto pps_it2 = received_pps_.find(nalu.pps_id); - if (pps_it2 == received_pps_.end()) { - return false; - } - sps_needed = pps_it2->second; - } - if (new_sps.find(sps_needed) == new_sps.end() && - received_sps_.find(sps_needed) == received_sps_.end()) { - return false; - } - break; - } - } - } - return true; -} - } // namespace webrtc diff --git a/webrtc/modules/video_coding/decoding_state.h b/webrtc/modules/video_coding/decoding_state.h index c7340809a7..f4ea8ae081 100644 --- a/webrtc/modules/video_coding/decoding_state.h +++ b/webrtc/modules/video_coding/decoding_state.h @@ -11,16 +11,11 @@ #ifndef WEBRTC_MODULES_VIDEO_CODING_DECODING_STATE_H_ #define WEBRTC_MODULES_VIDEO_CODING_DECODING_STATE_H_ -#include -#include -#include - #include "webrtc/typedefs.h" namespace webrtc { // Forward declarations -struct NaluInfo; class VCMFrameBuffer; class VCMPacket; @@ -66,7 +61,6 @@ class VCMDecodingState { bool UsingPictureId(const VCMFrameBuffer* frame) const; bool UsingFlexibleMode(const VCMFrameBuffer* frame) const; bool AheadOfFramesDecodedClearedTo(uint16_t index) const; - bool HaveSpsAndPps(const std::vector& nalus) const; // Keep state of last decoded frame. // TODO(mikhal/stefan): create designated classes to handle these types. @@ -81,8 +75,6 @@ class VCMDecodingState { // Used to check references in flexible mode. bool frame_decoded_[kFrameDecodedLength]; uint16_t frame_decoded_cleared_to_; - std::set received_sps_; - std::map received_pps_; }; } // namespace webrtc diff --git a/webrtc/modules/video_coding/frame_buffer.cc b/webrtc/modules/video_coding/frame_buffer.cc index bb8ec93713..b332a00c8d 100644 --- a/webrtc/modules/video_coding/frame_buffer.cc +++ b/webrtc/modules/video_coding/frame_buffer.cc @@ -66,10 +66,6 @@ bool VCMFrameBuffer::NonReference() const { return _sessionInfo.NonReference(); } -std::vector VCMFrameBuffer::GetNaluInfos() const { - return _sessionInfo.GetNaluInfos(); -} - void VCMFrameBuffer::SetGofInfo(const GofInfoVP9& gof_info, size_t idx) { _sessionInfo.SetGofInfo(gof_info, idx); // TODO(asapersson): Consider adding hdr->VP9.ref_picture_id for testing. diff --git a/webrtc/modules/video_coding/frame_buffer.h b/webrtc/modules/video_coding/frame_buffer.h index 95c598534c..f5a707efe4 100644 --- a/webrtc/modules/video_coding/frame_buffer.h +++ b/webrtc/modules/video_coding/frame_buffer.h @@ -11,8 +11,6 @@ #ifndef WEBRTC_MODULES_VIDEO_CODING_FRAME_BUFFER_H_ #define WEBRTC_MODULES_VIDEO_CODING_FRAME_BUFFER_H_ -#include - #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/video_coding/include/video_coding.h" #include "webrtc/modules/video_coding/encoded_frame.h" @@ -63,8 +61,6 @@ class VCMFrameBuffer : public VCMEncodedFrame { int Tl0PicId() const; bool NonReference() const; - std::vector GetNaluInfos() const; - void SetGofInfo(const GofInfoVP9& gof_info, size_t idx); // Increments a counter to keep track of the number of packets of this frame diff --git a/webrtc/modules/video_coding/jitter_buffer_unittest.cc b/webrtc/modules/video_coding/jitter_buffer_unittest.cc index 425a4e9745..c7108c11d8 100644 --- a/webrtc/modules/video_coding/jitter_buffer_unittest.cc +++ b/webrtc/modules/video_coding/jitter_buffer_unittest.cc @@ -12,9 +12,9 @@ #include #include -#include -#include "webrtc/common_video/h264/h264_common.h" +#include "webrtc/test/gtest.h" +#include "webrtc/test/gmock.h" #include "webrtc/modules/video_coding/frame_buffer.h" #include "webrtc/modules/video_coding/jitter_buffer.h" #include "webrtc/modules/video_coding/media_opt_util.h" @@ -26,8 +26,6 @@ #include "webrtc/system_wrappers/include/metrics.h" #include "webrtc/system_wrappers/include/metrics_default.h" #include "webrtc/test/field_trial.h" -#include "webrtc/test/gmock.h" -#include "webrtc/test/gtest.h" namespace webrtc { @@ -1162,89 +1160,6 @@ TEST_F(TestBasicJitterBuffer, H264InsertStartCode) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, SpsAndPpsHandling) { - jitter_buffer_->SetDecodeErrorMode(kNoErrors); - - packet_->timestamp = timestamp_; - packet_->frameType = kVideoFrameKey; - packet_->isFirstPacket = true; - packet_->markerBit = true; - packet_->codec = kVideoCodecH264; - packet_->video_header.codec = kRtpVideoH264; - packet_->video_header.codecHeader.H264.nalu_type = H264::NaluType::kIdr; - packet_->video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kIdr; - packet_->video_header.codecHeader.H264.nalus[0].sps_id = -1; - packet_->video_header.codecHeader.H264.nalus[0].pps_id = 0; - packet_->video_header.codecHeader.H264.nalus_length = 1; - bool retransmitted = false; - EXPECT_EQ(kCompleteSession, - jitter_buffer_->InsertPacket(*packet_, &retransmitted)); - // Not decodable since sps and pps are missing. - EXPECT_EQ(nullptr, DecodeCompleteFrame()); - - timestamp_ += 3000; - packet_->timestamp = timestamp_; - ++seq_num_; - packet_->seqNum = seq_num_; - packet_->frameType = kVideoFrameKey; - packet_->isFirstPacket = true; - packet_->markerBit = false; - packet_->codec = kVideoCodecH264; - packet_->video_header.codec = kRtpVideoH264; - packet_->video_header.codecHeader.H264.nalu_type = H264::NaluType::kStapA; - packet_->video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kSps; - packet_->video_header.codecHeader.H264.nalus[0].sps_id = 0; - packet_->video_header.codecHeader.H264.nalus[0].pps_id = -1; - packet_->video_header.codecHeader.H264.nalus[1].type = H264::NaluType::kPps; - packet_->video_header.codecHeader.H264.nalus[1].sps_id = 0; - packet_->video_header.codecHeader.H264.nalus[1].pps_id = 0; - packet_->video_header.codecHeader.H264.nalus_length = 2; - // Not complete since the marker bit hasn't been received. - EXPECT_EQ(kIncomplete, - jitter_buffer_->InsertPacket(*packet_, &retransmitted)); - - ++seq_num_; - packet_->seqNum = seq_num_; - packet_->frameType = kVideoFrameKey; - packet_->isFirstPacket = false; - packet_->markerBit = true; - packet_->codec = kVideoCodecH264; - packet_->video_header.codec = kRtpVideoH264; - packet_->video_header.codecHeader.H264.nalu_type = H264::NaluType::kIdr; - packet_->video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kIdr; - packet_->video_header.codecHeader.H264.nalus[0].sps_id = -1; - packet_->video_header.codecHeader.H264.nalus[0].pps_id = 0; - packet_->video_header.codecHeader.H264.nalus_length = 1; - // Complete and decodable since the pps and sps are received in the first - // packet of this frame. - EXPECT_EQ(kCompleteSession, - jitter_buffer_->InsertPacket(*packet_, &retransmitted)); - VCMEncodedFrame* frame_out = DecodeCompleteFrame(); - ASSERT_NE(nullptr, frame_out); - jitter_buffer_->ReleaseFrame(frame_out); - - timestamp_ += 3000; - packet_->timestamp = timestamp_; - ++seq_num_; - packet_->seqNum = seq_num_; - packet_->frameType = kVideoFrameDelta; - packet_->isFirstPacket = true; - packet_->markerBit = true; - packet_->codec = kVideoCodecH264; - packet_->video_header.codec = kRtpVideoH264; - packet_->video_header.codecHeader.H264.nalu_type = H264::NaluType::kSlice; - packet_->video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kSlice; - packet_->video_header.codecHeader.H264.nalus[0].sps_id = -1; - packet_->video_header.codecHeader.H264.nalus[0].pps_id = 0; - packet_->video_header.codecHeader.H264.nalus_length = 1; - // Complete and decodable since sps, pps and key frame has been received. - EXPECT_EQ(kCompleteSession, - jitter_buffer_->InsertPacket(*packet_, &retransmitted)); - frame_out = DecodeCompleteFrame(); - ASSERT_NE(nullptr, frame_out); - jitter_buffer_->ReleaseFrame(frame_out); -} - // Test threshold conditions of decodable state. TEST_F(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsThresholdCheck) { jitter_buffer_->SetDecodeErrorMode(kSelectiveErrors); @@ -2718,4 +2633,5 @@ TEST_F(TestJitterBufferNack, ResetByFutureKeyFrameDoesntError) { nack_list = jitter_buffer_->GetNackList(&extended); EXPECT_EQ(0u, nack_list.size()); } + } // namespace webrtc diff --git a/webrtc/modules/video_coding/session_info.cc b/webrtc/modules/video_coding/session_info.cc index 39c1c0b5b6..8f136eecb4 100644 --- a/webrtc/modules/video_coding/session_info.cc +++ b/webrtc/modules/video_coding/session_info.cc @@ -111,19 +111,6 @@ bool VCMSessionInfo::NonReference() const { return packets_.front().video_header.codecHeader.VP8.nonReference; } -std::vector VCMSessionInfo::GetNaluInfos() const { - if (packets_.empty() || packets_.front().video_header.codec != kRtpVideoH264) - return std::vector(); - std::vector nalu_infos; - for (const VCMPacket& packet : packets_) { - for (size_t i = 0; i < packet.video_header.codecHeader.H264.nalus_length; - ++i) { - nalu_infos.push_back(packet.video_header.codecHeader.H264.nalus[i]); - } - } - return nalu_infos; -} - void VCMSessionInfo::SetGofInfo(const GofInfoVP9& gof_info, size_t idx) { if (packets_.empty() || packets_.front().video_header.codec != kRtpVideoVp9 || packets_.front().video_header.codecHeader.VP9.flexible_mode) { diff --git a/webrtc/modules/video_coding/session_info.h b/webrtc/modules/video_coding/session_info.h index a4eb405d0b..c2de1acfde 100644 --- a/webrtc/modules/video_coding/session_info.h +++ b/webrtc/modules/video_coding/session_info.h @@ -12,7 +12,6 @@ #define WEBRTC_MODULES_VIDEO_CODING_SESSION_INFO_H_ #include -#include #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/video_coding/include/video_coding.h" @@ -82,8 +81,6 @@ class VCMSessionInfo { int Tl0PicId() const; bool NonReference() const; - std::vector GetNaluInfos() const; - void SetGofInfo(const GofInfoVP9& gof_info, size_t idx); // The number of packets discarded because the decoder can't make use of