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}
diff --git a/webrtc/modules/video_coding/decoding_state.cc b/webrtc/modules/video_coding/decoding_state.cc
index 530d167..89be9b6 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 @@
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 @@
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 @@
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 @@
// 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 @@
return ContinuousPictureId(frame->PictureId());
}
} else {
- return ContinuousSeqNum(static_cast<uint16_t>(frame->GetLowSeqNum())) &&
- HaveSpsAndPps(frame->GetNaluInfos());
+ return ContinuousSeqNum(static_cast<uint16_t>(frame->GetLowSeqNum()));
}
}
@@ -309,48 +282,4 @@
return diff > kFrameDecodedLength / 2;
}
-bool VCMDecodingState::HaveSpsAndPps(const std::vector<NaluInfo>& nalus) const {
- std::set<int> new_sps;
- std::map<int, int> 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 c734080..f4ea8ae 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 <map>
-#include <set>
-#include <vector>
-
#include "webrtc/typedefs.h"
namespace webrtc {
// Forward declarations
-struct NaluInfo;
class VCMFrameBuffer;
class VCMPacket;
@@ -66,7 +61,6 @@
bool UsingPictureId(const VCMFrameBuffer* frame) const;
bool UsingFlexibleMode(const VCMFrameBuffer* frame) const;
bool AheadOfFramesDecodedClearedTo(uint16_t index) const;
- bool HaveSpsAndPps(const std::vector<NaluInfo>& nalus) const;
// Keep state of last decoded frame.
// TODO(mikhal/stefan): create designated classes to handle these types.
@@ -81,8 +75,6 @@
// Used to check references in flexible mode.
bool frame_decoded_[kFrameDecodedLength];
uint16_t frame_decoded_cleared_to_;
- std::set<int> received_sps_;
- std::map<int, int> received_pps_;
};
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/frame_buffer.cc b/webrtc/modules/video_coding/frame_buffer.cc
index bb8ec93..b332a00c 100644
--- a/webrtc/modules/video_coding/frame_buffer.cc
+++ b/webrtc/modules/video_coding/frame_buffer.cc
@@ -66,10 +66,6 @@
return _sessionInfo.NonReference();
}
-std::vector<NaluInfo> 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 95c5985..f5a707e 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 <vector>
-
#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 @@
int Tl0PicId() const;
bool NonReference() const;
- std::vector<NaluInfo> 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 425a4e9..c7108c1 100644
--- a/webrtc/modules/video_coding/jitter_buffer_unittest.cc
+++ b/webrtc/modules/video_coding/jitter_buffer_unittest.cc
@@ -12,9 +12,9 @@
#include <list>
#include <memory>
-#include <vector>
-#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 @@
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 @@
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 39c1c0b..8f136ee 100644
--- a/webrtc/modules/video_coding/session_info.cc
+++ b/webrtc/modules/video_coding/session_info.cc
@@ -111,19 +111,6 @@
return packets_.front().video_header.codecHeader.VP8.nonReference;
}
-std::vector<NaluInfo> VCMSessionInfo::GetNaluInfos() const {
- if (packets_.empty() || packets_.front().video_header.codec != kRtpVideoH264)
- return std::vector<NaluInfo>();
- std::vector<NaluInfo> 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 a4eb405..c2de1ac 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 <list>
-#include <vector>
#include "webrtc/modules/include/module_common_types.h"
#include "webrtc/modules/video_coding/include/video_coding.h"
@@ -82,8 +81,6 @@
int Tl0PicId() const;
bool NonReference() const;
- std::vector<NaluInfo> GetNaluInfos() const;
-
void SetGofInfo(const GofInfoVP9& gof_info, size_t idx);
// The number of packets discarded because the decoder can't make use of