Reland of Ignore Camera and Flip bits in CVO when parsing video rotation (patchset #1 id:1 of https://codereview.webrtc.org/2300323002/ )

Reason for revert:
Downstream build is fixed.

Original issue's description:
> Revert of Ignore Camera and Flip bits in CVO when parsing video rotation (patchset #3 id:80001 of https://codereview.webrtc.org/2280703002/ )
>
> Reason for revert:
> Breaks downstream build.
>
> Original issue's description:
> > 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
> >
> > Committed: f9e1b922ef
>
> TBR=pthatcher@webrtc.org,danilchap@webrtc.org,tommi@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:6120
>
> Committed: https://crrev.com/97667c7746282704acccd896e26175decee349c0
> Cr-Commit-Position: refs/heads/master@{#14035}

TBR=pthatcher@webrtc.org,danilchap@webrtc.org,tommi@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:6120

Review-Url: https://codereview.webrtc.org/2320913003
Cr-Commit-Position: refs/heads/master@{#14124}

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)
@@ -697,7 +698,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

@@ -266,8 +266,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);
   }
 };
 
@@ -676,8 +675,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.
@@ -1790,4 +1788,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;