Ignore Camera and Flip bits in CVO when parsing video rotation

Currently, if WebRTC receives a CVO byte where the Camera or Flip bit is
set, then rotation is incorrectly parsed as 0. This CL fixes that issue.
The Camera and Flip bit is still unimplemented and will just be ignored
though.

BUG=webrtc:6120
R=danilchap@webrtc.org, pthatcher@webrtc.org, tommi@webrtc.org

Review URL: https://codereview.webrtc.org/2280703002 .

Cr-Commit-Position: refs/heads/master@{#14027}

webrtc/DEPS

@@ -13,6 +13,7 @@ include_rules = [
   "+webrtc/call.h",
   "+webrtc/common.h",
   "+webrtc/common_types.h",
+  "+webrtc/common_video/rotation.h",
   "+webrtc/config.h",
   "+webrtc/engine_configurations.h",
   "+webrtc/transport.h",

webrtc/common_types.cc

@@ -27,7 +27,7 @@ RTPHeaderExtension::RTPHeaderExtension()
       voiceActivity(false),
       audioLevel(0),
       hasVideoRotation(false),
-      videoRotation(0) {}
+      videoRotation(kVideoRotation_0) {}
 
 RTPHeader::RTPHeader()
     : markerBit(false),

webrtc/common_types.h

@@ -18,6 +18,7 @@
 #include <string>
 #include <vector>
 
+#include "webrtc/common_video/rotation.h"
 #include "webrtc/typedefs.h"
 
 #if defined(_MSC_VER)
@@ -699,7 +700,7 @@ struct RTPHeaderExtension {
   // http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/
   // ts_126114v120700p.pdf
   bool hasVideoRotation;
-  uint8_t videoRotation;
+  VideoRotation videoRotation;
 
   PlayoutDelay playout_delay = {-1, -1};
 };

webrtc/modules/rtp_rtcp/include/rtp_cvo.h

@@ -34,8 +34,10 @@ inline uint8_t ConvertVideoRotationToCVOByte(VideoRotation rotation) {
   return 0;
 }
 
-inline VideoRotation ConvertCVOByteToVideoRotation(uint8_t rotation) {
-  switch (rotation) {
+inline VideoRotation ConvertCVOByteToVideoRotation(uint8_t cvo_byte) {
+  // CVO byte: |0 0 0 0 C F R R|.
+  const uint8_t rotation_bits = cvo_byte & 0x3;
+  switch (rotation_bits) {
     case 0:
       return kVideoRotation_0;
     case 1:

webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc

@@ -177,7 +177,7 @@ bool VideoOrientation::IsSupportedFor(MediaType type) {
 }
 
 bool VideoOrientation::Parse(const uint8_t* data, VideoRotation* rotation) {
-  *rotation = ConvertCVOByteToVideoRotation(data[0] & 0x03);
+  *rotation = ConvertCVOByteToVideoRotation(data[0]);
   return true;
 }
 

webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc

@@ -94,8 +94,8 @@ int32_t RTPReceiverVideo::ParseRtpPacket(WebRtcRTPHeader* rtp_header,
 
   // Retrieve the video rotation information.
   if (rtp_header->header.extension.hasVideoRotation) {
-    rtp_header->type.Video.rotation = ConvertCVOByteToVideoRotation(
-        rtp_header->header.extension.videoRotation);
+    rtp_header->type.Video.rotation =
+        rtp_header->header.extension.videoRotation;
   }
 
   rtp_header->type.Video.playout_delay =

webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -262,8 +262,7 @@ class RtpSenderVideoTest : public RtpSenderTest {
     EXPECT_EQ(rtp_sender_->SSRC(), rtp_header.ssrc);
     EXPECT_EQ(0, rtp_header.numCSRCs);
     EXPECT_EQ(0U, rtp_header.paddingLength);
-    EXPECT_EQ(ConvertVideoRotationToCVOByte(rotation),
-              rtp_header.extension.videoRotation);
+    EXPECT_EQ(rotation, rtp_header.extension.videoRotation);
   }
 };
 
@@ -577,8 +576,7 @@ TEST_F(RtpSenderTestWithoutPacer, BuildRTPPacketWithVideoRotation_MarkerBit) {
   VerifyRTPHeaderCommon(rtp_header);
   EXPECT_EQ(length, rtp_header.headerLength);
   EXPECT_TRUE(rtp_header.extension.hasVideoRotation);
-  EXPECT_EQ(ConvertVideoRotationToCVOByte(kRotation),
-            rtp_header.extension.videoRotation);
+  EXPECT_EQ(kRotation, rtp_header.extension.videoRotation);
 }
 
 // Test CVO header extension is not set when marker bit is false.
@@ -1691,4 +1689,26 @@ TEST_F(RtpSenderVideoTest, SendVideoWithCVO) {
       transport_.sent_packets_[1]->size(), true, &map, kSeqNum + 1,
       hdr.rotation);
 }
+
+// Make sure rotation is parsed correctly when the Camera (C) and Flip (F) bits
+// are set in the CVO byte.
+TEST_F(RtpSenderVideoTest, SendVideoWithCameraAndFlipCVO) {
+  // Test extracting rotation when Camera (C) and Flip (F) bits are zero.
+  EXPECT_EQ(kVideoRotation_0, ConvertCVOByteToVideoRotation(0));
+  EXPECT_EQ(kVideoRotation_90, ConvertCVOByteToVideoRotation(1));
+  EXPECT_EQ(kVideoRotation_180, ConvertCVOByteToVideoRotation(2));
+  EXPECT_EQ(kVideoRotation_270, ConvertCVOByteToVideoRotation(3));
+  // Test extracting rotation when Camera (C) and Flip (F) bits are set.
+  const int flip_bit = 1 << 2;
+  const int camera_bit = 1 << 3;
+  EXPECT_EQ(kVideoRotation_0,
+            ConvertCVOByteToVideoRotation(flip_bit | camera_bit | 0));
+  EXPECT_EQ(kVideoRotation_90,
+            ConvertCVOByteToVideoRotation(flip_bit | camera_bit | 1));
+  EXPECT_EQ(kVideoRotation_180,
+            ConvertCVOByteToVideoRotation(flip_bit | camera_bit | 2));
+  EXPECT_EQ(kVideoRotation_270,
+            ConvertCVOByteToVideoRotation(flip_bit | camera_bit | 3));
+}
+
 }  // namespace webrtc

webrtc/modules/rtp_rtcp/source/rtp_utility.cc

@@ -13,6 +13,7 @@
 #include <string.h>
 
 #include "webrtc/base/logging.h"
+#include "webrtc/modules/rtp_rtcp/include/rtp_cvo.h"
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 
 namespace webrtc {
@@ -246,7 +247,7 @@ bool RtpHeaderParser::Parse(RTPHeader* header,
 
   // May not be present in packet.
   header->extension.hasVideoRotation = false;
-  header->extension.videoRotation = 0;
+  header->extension.videoRotation = kVideoRotation_0;
 
   // May not be present in packet.
   header->extension.playout_delay.min_ms = -1;
@@ -397,7 +398,8 @@ void RtpHeaderParser::ParseOneByteExtensionHeader(
           // |  ID   | len=0 |0 0 0 0 C F R R|
           // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
           header->extension.hasVideoRotation = true;
-          header->extension.videoRotation = ptr[0];
+          header->extension.videoRotation =
+              ConvertCVOByteToVideoRotation(ptr[0]);
           break;
         }
         case kRtpExtensionTransportSequenceNumber: {

webrtc/sdk/DEPS

@@ -2,7 +2,6 @@ include_rules = [
   "+WebRTC",
   "+webrtc/api",
   "+webrtc/common_video/include",
-  "+webrtc/common_video/rotation.h",
   "+webrtc/media",
   "+webrtc/system_wrappers",
 ]

webrtc/video/rtp_stream_receiver.cc

@@ -443,8 +443,7 @@ void RtpStreamReceiver::NotifyReceiverOfFecPacket(const RTPHeader& header) {
   rtp_header.type.Video.codec = payload_specific.Video.videoCodecType;
   rtp_header.type.Video.rotation = kVideoRotation_0;
   if (header.extension.hasVideoRotation) {
-    rtp_header.type.Video.rotation =
-        ConvertCVOByteToVideoRotation(header.extension.videoRotation);
+    rtp_header.type.Video.rotation = header.extension.videoRotation;
   }
   rtp_header.type.Video.playout_delay = header.extension.playout_delay;