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: https://chromium.googlesource.com/external/webrtc/+/f9e1b922ef3e0cbe70953dfb7a1d4cb2c44a49e3
>
> 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}
diff --git a/webrtc/DEPS b/webrtc/DEPS
index 41e0b71..ffb67d1 100644
--- a/webrtc/DEPS
+++ b/webrtc/DEPS
@@ -13,6 +13,7 @@
"+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",
diff --git a/webrtc/common_types.cc b/webrtc/common_types.cc
index 86e7813..8f117e1 100644
--- a/webrtc/common_types.cc
+++ b/webrtc/common_types.cc
@@ -27,7 +27,7 @@
voiceActivity(false),
audioLevel(0),
hasVideoRotation(false),
- videoRotation(0) {}
+ videoRotation(kVideoRotation_0) {}
RTPHeader::RTPHeader()
: markerBit(false),
diff --git a/webrtc/common_types.h b/webrtc/common_types.h
index ea040fd..6d30719 100644
--- a/webrtc/common_types.h
+++ b/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 @@
// 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};
};
diff --git a/webrtc/modules/rtp_rtcp/include/rtp_cvo.h b/webrtc/modules/rtp_rtcp/include/rtp_cvo.h
index 5d5c995..b60b06e 100644
--- a/webrtc/modules/rtp_rtcp/include/rtp_cvo.h
+++ b/webrtc/modules/rtp_rtcp/include/rtp_cvo.h
@@ -34,8 +34,10 @@
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:
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc b/webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc
index a851bc3..0d4ce95 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc
@@ -177,7 +177,7 @@
}
bool VideoOrientation::Parse(const uint8_t* data, VideoRotation* rotation) {
- *rotation = ConvertCVOByteToVideoRotation(data[0] & 0x03);
+ *rotation = ConvertCVOByteToVideoRotation(data[0]);
return true;
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc
index 7667c46..9f6d80a 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc
@@ -94,8 +94,8 @@
// 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 =
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index d4cba75..811ac77 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -266,8 +266,7 @@
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 @@
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 @@
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
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_utility.cc b/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
index e38b3a3..7dd5973 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
+++ b/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 @@
// 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 @@
// | 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: {
diff --git a/webrtc/sdk/DEPS b/webrtc/sdk/DEPS
index 16ef0f6..d6e53d5 100644
--- a/webrtc/sdk/DEPS
+++ b/webrtc/sdk/DEPS
@@ -2,7 +2,6 @@
"+WebRTC",
"+webrtc/api",
"+webrtc/common_video/include",
- "+webrtc/common_video/rotation.h",
"+webrtc/media",
"+webrtc/system_wrappers",
]
diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc
index d5b547f..20acc06 100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -443,8 +443,7 @@
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;
