Unit test out of band H264 SPS,PPS within RtpStreamReceiver.

webrtc/video/rtp_stream_receiver_unittest.cc

+/*
+ *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/test/gtest.h"
+#include "webrtc/test/gmock.h"
+
+#include "webrtc/base/logging.h"
+#include "webrtc/common_video/h264/h264_common.h"
+#include "webrtc/media/base/mediaconstants.h"
+#include "webrtc/modules/pacing/packet_router.h"
+#include "webrtc/modules/video_coding/include/video_coding_defines.h"
+#include "webrtc/modules/video_coding/frame_object.h"
+#include "webrtc/modules/video_coding/packet.h"
+#include "webrtc/modules/video_coding/rtp_frame_reference_finder.h"
+#include "webrtc/modules/video_coding/timing.h"
+#include "webrtc/modules/utility/include/process_thread.h"
+#include "webrtc/system_wrappers/include/clock.h"
+#include "webrtc/system_wrappers/include/field_trial_default.h"
+#include "webrtc/video/rtp_stream_receiver.h"
+
+namespace webrtc {
+
+namespace {
+
+const char new_jb_enabled[] = "WebRTC-NewVideoJitterBuffer/Enabled/";

[sprang_webrtc, 2017/01/18 10:05:11]

I'd change the name to something like kNewJitterBufferFieldTrialEnabled just to
be extra obvious.

+
+class MockTransport : public Transport {
+ public:
+  MOCK_METHOD3(SendRtp,
+               bool(const uint8_t* packet,
+                    size_t length,
+                    const PacketOptions& options));
+  MOCK_METHOD2(SendRtcp, bool(const uint8_t* packet, size_t length));
+};
+
+class MockNackSender : public NackSender {
+ public:
+  MOCK_METHOD1(SendNack, void(const std::vector<uint16_t>& sequence_numbers));
+};
+
+class MockKeyFrameRequestSender : public KeyFrameRequestSender {
+ public:
+  MOCK_METHOD0(RequestKeyFrame, void());
+};
+
+class MockOnCompleteFrameCallback
+    : public video_coding::OnCompleteFrameCallback {
+ public:
+  MOCK_METHOD1(DoOnCompleteFrame, void(video_coding::FrameObject* frame));
+  void OnCompleteFrame(std::unique_ptr<video_coding::FrameObject> frame) {
+    DoOnCompleteFrame(frame.get());
+  }
+};
+
+}  // namespace
+
+class RtpStreamReceiverTest : public testing::Test {
+ public:
+  RtpStreamReceiverTest()
+      : config_(CreateConfig()),
+        timing_(Clock::GetRealTimeClock()),
+        process_thread_(ProcessThread::Create("TestThread")) {
+    // InitFieldTrials has to done before creation of rtp_stream_receiver_.

[sprang_webrtc, 2017/01/18 10:05:10]

nit: s/has to done/has to be done
Or you can move rtp_stream_receiver_.reset(...) to the SetUp() method.

[johan, 2017/01/18 12:38:29]

Ack
Well, reviewers of other modules asked me to do stuff in constructors instead of
SetUp() methods, whenever possible. Would be nice to have a common guideline
here...

[sprang_webrtc, 2017/01/23 12:57:09]

I don't have super strong preferences, but if the initialization ordering risks
breaking stuff I would lean towards a way of making it quite clear.

You could also look into using a ScopedFieldTrials as member, then you could
bring this all back into the initializer list.

+    field_trial::InitFieldTrialsFromString(new_jb_enabled);
+    rtp_stream_receiver_.reset(new RtpStreamReceiver(
+        nullptr, nullptr, &mock_transport_, nullptr, nullptr, &packet_router_,
+        nullptr, &config_, nullptr, process_thread_.get(), nullptr,
+        &mock_nack_sender_, &mock_key_frame_request_sender_,
+        &mock_on_complete_frame_callback_, &timing_));
+  }
+
+  WebRtcRTPHeader GetDefaultPacket() {
+    WebRtcRTPHeader packet;
+    memset(&packet, 0, sizeof(packet));
+    packet.type.Video.codec = kRtpVideoH264;
+    return packet;
+  }
+
+  // TODO(Johan): refactor h264_sps_pps_tracker_unittests.cc to avoid duplicate
+  // code.
+  void AddSps(WebRtcRTPHeader* packet, int sps_id, std::vector<uint8_t>* data) {
+    NaluInfo info;
+    info.type = H264::NaluType::kSps;
+    info.sps_id = sps_id;
+    info.pps_id = -1;
+    info.offset = data->size();
+    info.size = 2;
+    data->push_back(H264::NaluType::kSps);
+    data->push_back(sps_id);
+    packet->type.Video.codecHeader.H264
+        .nalus[packet->type.Video.codecHeader.H264.nalus_length++] = info;
+  }
+
+  void AddPps(WebRtcRTPHeader* packet,
+              int sps_id,
+              int pps_id,
+              std::vector<uint8_t>* data) {
+    NaluInfo info;
+    info.type = H264::NaluType::kPps;
+    info.sps_id = sps_id;
+    info.pps_id = pps_id;
+    info.offset = data->size();
+    info.size = 2;
+    data->push_back(H264::NaluType::kPps);
+    data->push_back(pps_id);
+    packet->type.Video.codecHeader.H264
+        .nalus[packet->type.Video.codecHeader.H264.nalus_length++] = info;
+  }
+
+  void AddIdr(WebRtcRTPHeader* packet, int pps_id) {
+    NaluInfo info;
+    info.type = H264::NaluType::kIdr;
+    info.sps_id = -1;
+    info.pps_id = pps_id;
+    packet->type.Video.codecHeader.H264
+        .nalus[packet->type.Video.codecHeader.H264.nalus_length++] = info;
+  }
+
+ protected:
+  static VideoReceiveStream::Config CreateConfig() {
+    VideoReceiveStream::Config config(nullptr);
+    config.rtp.remote_ssrc = 1111;
+    config.rtp.local_ssrc = 2222;
+    return config;
+  }

[sprang_webrtc, 2017/01/18 10:05:10]

nit: \n

+  VideoReceiveStream::Config config_;
+  MockNackSender mock_nack_sender_;
+  MockKeyFrameRequestSender mock_key_frame_request_sender_;
+  MockTransport mock_transport_;
+  MockOnCompleteFrameCallback mock_on_complete_frame_callback_;
+  PacketRouter packet_router_;
+  VCMTiming timing_;
+  std::unique_ptr<ProcessThread> process_thread_;
+  std::unique_ptr<RtpStreamReceiver> rtp_stream_receiver_;
+};
+
+TEST_F(RtpStreamReceiverTest, GenericKeyFrame) {
+  WebRtcRTPHeader rtp_header;
+  const std::vector<uint8_t> data({1, 2, 3, 4});
+  memset(&rtp_header, 0, sizeof(rtp_header));
+  rtp_header.header.sequenceNumber = 1;
+  rtp_header.header.markerBit = 1;
+  rtp_header.type.Video.is_first_packet_in_frame = true;
+  rtp_header.frameType = kVideoFrameKey;
+  rtp_header.type.Video.codec = kRtpVideoGeneric;
+  EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(testing::_));

[sprang_webrtc, 2017/01/18 10:05:10]

add "using ::testing::_;" above

[johan, 2017/01/18 12:38:30]

Acknowledged.

+  rtp_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(),
+                                              &rtp_header);
+}
+
+TEST_F(RtpStreamReceiverTest, InBandSpsPps) {
+  EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(testing::_));

[sprang_webrtc, 2017/01/18 10:05:10]

When do we expect it? If it's a result of
rtp_stream_receiver_->OnReceivedPayloadData(), I suggest moving the EXPECT_CALL
to just before that.

Can we determine what the frame will contain, so that we can use actual data
instead of _ ?

[johan, 2017/01/18 12:38:30]

OK
It should be possible to determine the expected Frame's bitstream. I will create
a patchset for the other comments first, and  figure out how to verify data in
this callback afterwards.

+  std::vector<uint8_t> data;
+  WebRtcRTPHeader sps_pps_packet = GetDefaultPacket();
+
+  AddSps(&sps_pps_packet, 0, &data);
+  sps_pps_packet.header.sequenceNumber = 0;
+  rtp_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(),
+                                              &sps_pps_packet);
+  data.clear();
+  AddPps(&sps_pps_packet, 0, 1, &data);
+  sps_pps_packet.header.sequenceNumber = 1;
+  rtp_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(),
+                                              &sps_pps_packet);
+  data.clear();
+
+  WebRtcRTPHeader idr_packet = GetDefaultPacket();
+  AddIdr(&idr_packet, 1);
+  idr_packet.type.Video.is_first_packet_in_frame = true;
+  idr_packet.header.sequenceNumber = 2;
+  idr_packet.header.markerBit = 1;
+  idr_packet.type.Video.is_first_packet_in_frame = true;
+  idr_packet.frameType = kVideoFrameKey;
+  idr_packet.type.Video.codec = kRtpVideoH264;
+  data.insert(data.end(), {1, 2, 3});
+  rtp_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(),
+                                              &idr_packet);
+}
+
+TEST_F(RtpStreamReceiverTest, OutOfBandFmtpSpsPps) {
+  EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(testing::_));

[sprang_webrtc, 2017/01/18 10:05:10]

Same here

+  constexpr int payload_type = 99;

[sprang_webrtc, 2017/01/18 10:05:11]

kPayloadType

+  VideoCodec codec;
+  codec.plType = payload_type;
+  std::map<std::string, std::string> codec_params;
+  // Example parameter sets from https://tools.ietf.org/html/rfc3984#section-8.2
+  // .
+  codec_params.insert(
+      {cricket::kH264FmtpSpropParameterSets, "Z0IACpZTBYmI,aMljiA=="});
+  rtp_stream_receiver_->AddReceiveCodec(codec, codec_params);
+
+  std::vector<uint8_t> data;
+  WebRtcRTPHeader idr_packet = GetDefaultPacket();
+  AddIdr(&idr_packet, 0);
+  idr_packet.header.payloadType = payload_type;
+  idr_packet.type.Video.is_first_packet_in_frame = true;
+  idr_packet.header.sequenceNumber = 2;
+  idr_packet.header.markerBit = 1;
+  idr_packet.type.Video.is_first_packet_in_frame = true;
+  idr_packet.frameType = kVideoFrameKey;
+  idr_packet.type.Video.codec = kRtpVideoH264;
+  data.insert(data.end(), {1, 2, 3});
+  rtp_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(),
+                                              &idr_packet);
+}
+
+}  // namespace webrtc