From 47836b4ebb8a5c71695b5ec07bffd5ee4e3bc2ff Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Mon, 6 Nov 2017 11:49:19 +0100 Subject: [PATCH] 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 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} --- 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, 34 insertions(+), 10 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_format_vp9.cc b/modules/rtp_rtcp/source/rtp_format_vp9.cc index 89a9848c83..2623790f36 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp9.cc @@ -721,7 +721,6 @@ 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 468f77b821..c522c4dc8a 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc @@ -30,8 +30,7 @@ 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 == kNoSpatialIdx ? 0 : expected.spatial_idx, - actual.spatial_idx); + EXPECT_EQ(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 f417dde76e..93df93fda4 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -3114,8 +3114,23 @@ 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 - EXPECT_GE(vp9.spatial_idx, 0); // S - EXPECT_LT(vp9.spatial_idx, vp9_settings_.numberOfSpatialLayers); + + 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); + } + if (vp9.ss_data_available) // V VerifySsData(vp9); @@ -3257,16 +3272,27 @@ 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; + bool ss_data_expected = + !vp9.inter_pic_predicted && vp9.beginning_of_frame && + (vp9.spatial_idx == 0 || vp9.spatial_idx == kNoSpatialIdx); EXPECT_EQ(ss_data_expected, vp9.ss_data_available); - EXPECT_EQ(vp9.spatial_idx > 0, vp9.inter_layer_predicted); // D + 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.inter_pic_predicted, frames_sent_ % kKeyFrameInterval == 0); if (IsNewPictureId(vp9)) { - EXPECT_EQ(0, vp9.spatial_idx); - EXPECT_EQ(num_spatial_layers_ - 1, last_vp9_.spatial_idx); + 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); } VerifyFixedTemporalLayerStructure(vp9,