From ae2942848940b807ec138adc1742a428980fabb4 Mon Sep 17 00:00:00 2001 From: Ivo Creusen Date: Mon, 6 Nov 2017 15:27:39 +0000 Subject: [PATCH] Revert "Keep spatial_idx=kNoSpatialIdx(255) if there is no layer indices." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 47836b4ebb8a5c71695b5ec07bffd5ee4e3bc2ff. Reason for revert: This breaks internal tests, reverting to check if they recover. Original change's description: > Keep spatial_idx=kNoSpatialIdx(255) if there is no layer indices. > > spatial_idx is not present in RTP header if there is no temporal or > spatial layering. But the parser sets spatial_idx to 0 in this case. > When reflector repacketizes such packets it writes layering indices > into outgoing packets. When packets arrive to receiver it thinks that > it deals with multi layer stream and passes it through special path > in Vp9 reference frame finder which never outputs inter frames. > > I modified the parser such that it keeps spatial_idx=kNoSpatialIdx(255) > when there is no layer indices in RTP header. Related unit tests have > been modified as well. > > Bug: none > Change-Id: I14498cafb4e57797577dc873298c35b243479f88 > Reviewed-on: https://webrtc-review.googlesource.com/17980 > Commit-Queue: Sergey Silkin > Reviewed-by: Erik Språng > Reviewed-by: Åsa Persson > Reviewed-by: Rasmus Brandt > Cr-Commit-Position: refs/heads/master@{#20560} TBR=brandtr@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org,ssilkin@webrtc.org Change-Id: I67d083cf769974d8df8bd5d70942af97db578db9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: none Reviewed-on: https://webrtc-review.googlesource.com/20501 Reviewed-by: Ivo Creusen Commit-Queue: Ivo Creusen Cr-Commit-Position: refs/heads/master@{#20565} --- modules/rtp_rtcp/source/rtp_format_vp9.cc | 1 + .../source/rtp_format_vp9_unittest.cc | 3 +- video/video_send_stream_tests.cc | 40 ++++--------------- 3 files changed, 10 insertions(+), 34 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_format_vp9.cc b/modules/rtp_rtcp/source/rtp_format_vp9.cc index 2623790f36..89a9848c83 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp9.cc @@ -721,6 +721,7 @@ bool RtpDepacketizerVp9::Parse(ParsedPayload* parsed_payload, vp9->beginning_of_frame = b_bit ? true : false; vp9->end_of_frame = e_bit ? true : false; vp9->ss_data_available = v_bit ? true : false; + vp9->spatial_idx = 0; // Parse fields that are present. if (i_bit && !ParsePictureId(&parser, vp9)) { diff --git a/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc b/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc index c522c4dc8a..468f77b821 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc @@ -30,7 +30,8 @@ void VerifyHeader(const RTPVideoHeaderVP9& expected, EXPECT_EQ(expected.picture_id, actual.picture_id); EXPECT_EQ(expected.max_picture_id, actual.max_picture_id); EXPECT_EQ(expected.temporal_idx, actual.temporal_idx); - EXPECT_EQ(expected.spatial_idx, actual.spatial_idx); + EXPECT_EQ(expected.spatial_idx == kNoSpatialIdx ? 0 : expected.spatial_idx, + actual.spatial_idx); EXPECT_EQ(expected.gof_idx, actual.gof_idx); EXPECT_EQ(expected.tl0_pic_idx, actual.tl0_pic_idx); EXPECT_EQ(expected.temporal_up_switch, actual.temporal_up_switch); diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 93df93fda4..f417dde76e 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -3114,23 +3114,8 @@ class Vp9HeaderObserver : public test::SendTest { EXPECT_EQ(kMaxTwoBytePictureId, vp9.max_picture_id); // M:1 EXPECT_NE(kNoPictureId, vp9.picture_id); // I:1 EXPECT_EQ(vp9_settings_.flexibleMode, vp9.flexible_mode); // F - - if (vp9_settings_.numberOfSpatialLayers > 1) { - EXPECT_LT(vp9.spatial_idx, vp9_settings_.numberOfSpatialLayers); - } else if (vp9_settings_.numberOfTemporalLayers > 1) { - EXPECT_EQ(vp9.spatial_idx, 0); - } else { - EXPECT_EQ(vp9.spatial_idx, kNoSpatialIdx); - } - - if (vp9_settings_.numberOfTemporalLayers > 1) { - EXPECT_LT(vp9.temporal_idx, vp9_settings_.numberOfTemporalLayers); - } else if (vp9_settings_.numberOfSpatialLayers > 1) { - EXPECT_EQ(vp9.temporal_idx, 0); - } else { - EXPECT_EQ(vp9.temporal_idx, kNoTemporalIdx); - } - + EXPECT_GE(vp9.spatial_idx, 0); // S + EXPECT_LT(vp9.spatial_idx, vp9_settings_.numberOfSpatialLayers); if (vp9.ss_data_available) // V VerifySsData(vp9); @@ -3272,27 +3257,16 @@ void VideoSendStreamTest::TestVp9NonFlexMode(uint8_t num_temporal_layers, } void InspectHeader(const RTPVideoHeaderVP9& vp9) override { - bool ss_data_expected = - !vp9.inter_pic_predicted && vp9.beginning_of_frame && - (vp9.spatial_idx == 0 || vp9.spatial_idx == kNoSpatialIdx); + bool ss_data_expected = !vp9.inter_pic_predicted && + vp9.beginning_of_frame && vp9.spatial_idx == 0; EXPECT_EQ(ss_data_expected, vp9.ss_data_available); - if (num_spatial_layers_ > 1) { - EXPECT_EQ(vp9.spatial_idx > 0, vp9.inter_layer_predicted); - } else { - EXPECT_FALSE(vp9.inter_layer_predicted); - } - + EXPECT_EQ(vp9.spatial_idx > 0, vp9.inter_layer_predicted); // D EXPECT_EQ(!vp9.inter_pic_predicted, frames_sent_ % kKeyFrameInterval == 0); if (IsNewPictureId(vp9)) { - if (num_temporal_layers_ == 1 && num_spatial_layers_ == 1) { - EXPECT_EQ(kNoSpatialIdx, vp9.spatial_idx); - } else { - EXPECT_EQ(0, vp9.spatial_idx); - } - if (num_spatial_layers_ > 1) - EXPECT_EQ(num_spatial_layers_ - 1, last_vp9_.spatial_idx); + EXPECT_EQ(0, vp9.spatial_idx); + EXPECT_EQ(num_spatial_layers_ - 1, last_vp9_.spatial_idx); } VerifyFixedTemporalLayerStructure(vp9,