From 2c37078e40d001d746686ebee86ce2969f3249d8 Mon Sep 17 00:00:00 2001
From: Guo-wei Shieh <guoweis@chromium.org>
Date: Wed, 8 Apr 2015 13:00:10 -0700
Subject: [PATCH] Fix crash with CVO turned on for VP9 codec

CopyCodecSpecific nulls out the rtpheader pointer hence causing the crash downstream.

More details about the codec type enums:
There are 2 enums defined. webrtc::VideoCodecType webrtc::RtpCodecTypes and they don't match. Inside CopyCodecSpecific in generic_encoder.cc, it was converted from the first to the 2nd type. At that point, it'll be kRtpVideoNone (as the effect of memset to 0). kRtpVideoNone is a bad value as it could cause assert. Later, it'll be reset to kRtpVideoGeneric in RTPSender::SendOutgoingData so it's not a concern.

BUG=4511
R=pbos@webrtc.org, pthatcher@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/29b1a1c0c7c6f4b1ae4d63844b1dfaa7a72530a0
Cr-Commit-Position: refs/heads/master@{#8951}

Review URL: https://webrtc-codereview.appspot.com/47999004

Cr-Commit-Position: refs/heads/master@{#8955}
---
 .../main/source/generic_encoder.cc            | 38 +++++++++----------
 webrtc/test/call_test.cc                      |  7 +++-
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/webrtc/modules/video_coding/main/source/generic_encoder.cc b/webrtc/modules/video_coding/main/source/generic_encoder.cc
index 0cc2ec4e8..58fdda19c 100644
--- a/webrtc/modules/video_coding/main/source/generic_encoder.cc
+++ b/webrtc/modules/video_coding/main/source/generic_encoder.cc
@@ -8,6 +8,7 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include "webrtc/base/checks.h"
 #include "webrtc/engine_configurations.h"
 #include "webrtc/modules/video_coding/main/source/encoded_frame.h"
 #include "webrtc/modules/video_coding/main/source/generic_encoder.h"
@@ -19,35 +20,30 @@ namespace webrtc {
 namespace {
 // Map information from info into rtp. If no relevant information is found
 // in info, rtp is set to NULL.
-void CopyCodecSpecific(const CodecSpecificInfo* info, RTPVideoHeader** rtp) {
-  if (!info) {
-    *rtp = NULL;
-    return;
-  }
+void CopyCodecSpecific(const CodecSpecificInfo* info, RTPVideoHeader* rtp) {
+  DCHECK(info);
   switch (info->codecType) {
     case kVideoCodecVP8: {
-      (*rtp)->codec = kRtpVideoVp8;
-      (*rtp)->codecHeader.VP8.InitRTPVideoHeaderVP8();
-      (*rtp)->codecHeader.VP8.pictureId = info->codecSpecific.VP8.pictureId;
-      (*rtp)->codecHeader.VP8.nonReference =
+      rtp->codec = kRtpVideoVp8;
+      rtp->codecHeader.VP8.InitRTPVideoHeaderVP8();
+      rtp->codecHeader.VP8.pictureId = info->codecSpecific.VP8.pictureId;
+      rtp->codecHeader.VP8.nonReference =
           info->codecSpecific.VP8.nonReference;
-      (*rtp)->codecHeader.VP8.temporalIdx = info->codecSpecific.VP8.temporalIdx;
-      (*rtp)->codecHeader.VP8.layerSync = info->codecSpecific.VP8.layerSync;
-      (*rtp)->codecHeader.VP8.tl0PicIdx = info->codecSpecific.VP8.tl0PicIdx;
-      (*rtp)->codecHeader.VP8.keyIdx = info->codecSpecific.VP8.keyIdx;
-      (*rtp)->simulcastIdx = info->codecSpecific.VP8.simulcastIdx;
+      rtp->codecHeader.VP8.temporalIdx = info->codecSpecific.VP8.temporalIdx;
+      rtp->codecHeader.VP8.layerSync = info->codecSpecific.VP8.layerSync;
+      rtp->codecHeader.VP8.tl0PicIdx = info->codecSpecific.VP8.tl0PicIdx;
+      rtp->codecHeader.VP8.keyIdx = info->codecSpecific.VP8.keyIdx;
+      rtp->simulcastIdx = info->codecSpecific.VP8.simulcastIdx;
       return;
     }
     case kVideoCodecH264:
-      (*rtp)->codec = kRtpVideoH264;
+      rtp->codec = kRtpVideoH264;
       return;
     case kVideoCodecGeneric:
-      (*rtp)->codec = kRtpVideoGeneric;
-      (*rtp)->simulcastIdx = info->codecSpecific.generic.simulcast_idx;
+      rtp->codec = kRtpVideoGeneric;
+      rtp->simulcastIdx = info->codecSpecific.generic.simulcast_idx;
       return;
     default:
-      // No codec specific info. Change RTP header pointer to NULL.
-      *rtp = NULL;
       return;
   }
 }
@@ -256,7 +252,9 @@ int32_t VCMEncodedFrameCallback::Encoded(
   RTPVideoHeader rtpVideoHeader;
   memset(&rtpVideoHeader, 0, sizeof(RTPVideoHeader));
   RTPVideoHeader* rtpVideoHeaderPtr = &rtpVideoHeader;
-  CopyCodecSpecific(codecSpecificInfo, &rtpVideoHeaderPtr);
+  if (codecSpecificInfo) {
+    CopyCodecSpecific(codecSpecificInfo, rtpVideoHeaderPtr);
+  }
   rtpVideoHeader.rotation = _rotation;
 
   int32_t callbackReturn = _sendCallback->SendData(
diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc
index 126c71635..9e78e82d7 100644
--- a/webrtc/test/call_test.cc
+++ b/webrtc/test/call_test.cc
@@ -8,12 +8,15 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 #include "webrtc/test/call_test.h"
-
 #include "webrtc/test/encoder_settings.h"
 
 namespace webrtc {
 namespace test {
 
+namespace {
+const int kVideoRotationRtpExtensionId = 4;
+}
+
 CallTest::CallTest()
     : clock_(Clock::GetRealTimeClock()),
       send_stream_(NULL),
@@ -94,6 +97,8 @@ void CallTest::CreateSendConfig(size_t num_streams) {
   encoder_config_.streams = test::CreateVideoStreams(num_streams);
   for (size_t i = 0; i < num_streams; ++i)
     send_config_.rtp.ssrcs.push_back(kSendSsrcs[i]);
+  send_config_.rtp.extensions.push_back(
+      RtpExtension(RtpExtension::kVideoRotation, kVideoRotationRtpExtensionId));
 }
 
 void CallTest::CreateMatchingReceiveConfigs() {