New equality operators, for structs related to webrtc::VideoCodec.
Added for the structs VideoCodecVP8, VideoCodecVP9, VideoCodecH264,
and SpatialLayer.
New operators are used to replace memcmp in VCMEncoderDataBase. Using
memcmp to compare structs is generally unreliable, since the struct
may contain random padding bytes due to alignment requirements
(affects at least VideoCodecH264). And in the case of VideoCodecVP8,
we need to exclude the tl_factory pointers from the comparison.
Bug: webrtc:8830
Change-Id: I40432ea7834e288f8c89ce0a28a630ae1800dff8
Reviewed-on: https://webrtc-review.googlesource.com/62761
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22500}diff --git a/common_types.cc b/common_types.cc
index 35987a0..0b262a6 100644
--- a/common_types.cc
+++ b/common_types.cc
@@ -20,6 +20,52 @@
namespace webrtc {
+bool VideoCodecVP8::operator==(const VideoCodecVP8& other) const {
+ // Doesn't compare the tl_factory pointers, which are constructed
+ // based on other members.
+ return (complexity == other.complexity &&
+ resilience == other.resilience &&
+ numberOfTemporalLayers == other.numberOfTemporalLayers &&
+ denoisingOn == other.denoisingOn &&
+ automaticResizeOn == other.automaticResizeOn &&
+ frameDroppingOn == other.frameDroppingOn &&
+ keyFrameInterval == other.keyFrameInterval);
+}
+
+bool VideoCodecVP9::operator==(const VideoCodecVP9& other) const {
+ return (complexity == other.complexity &&
+ resilienceOn == other.resilienceOn &&
+ numberOfTemporalLayers == other.numberOfTemporalLayers &&
+ denoisingOn == other.denoisingOn &&
+ frameDroppingOn == other.frameDroppingOn &&
+ keyFrameInterval == other.keyFrameInterval &&
+ adaptiveQpMode == other.adaptiveQpMode &&
+ automaticResizeOn == other.automaticResizeOn &&
+ numberOfSpatialLayers == other.numberOfSpatialLayers &&
+ flexibleMode == other.flexibleMode);
+}
+
+bool VideoCodecH264::operator==(const VideoCodecH264& other) const {
+ return (frameDroppingOn == other.frameDroppingOn &&
+ keyFrameInterval == other.keyFrameInterval &&
+ spsLen == other.spsLen &&
+ ppsLen == other.ppsLen &&
+ profile == other.profile &&
+ (spsLen == 0 || memcmp(spsData, other.spsData, spsLen) == 0) &&
+ (ppsLen == 0 || memcmp(ppsData, other.ppsData, ppsLen) == 0));
+}
+
+bool SpatialLayer::operator==(const SpatialLayer& other) const {
+ return (width == other.width &&
+ height == other.height &&
+ numberOfTemporalLayers == other.numberOfTemporalLayers &&
+ maxBitrate == other.maxBitrate &&
+ targetBitrate == other.targetBitrate &&
+ minBitrate == other.minBitrate &&
+ qpMax == other.qpMax &&
+ active == other.active);
+}
+
VideoCodec::VideoCodec()
: codecType(kVideoCodecUnknown),
plType(0),
diff --git a/common_types.h b/common_types.h
index ee4ddf7..482cb04 100644
--- a/common_types.h
+++ b/common_types.h
@@ -422,6 +422,10 @@
class TemporalLayersFactory;
// VP8 specific
struct VideoCodecVP8 {
+ bool operator==(const VideoCodecVP8& other) const;
+ bool operator!=(const VideoCodecVP8& other) const {
+ return !(*this == other);
+ }
VideoCodecComplexity complexity;
VP8ResilienceMode resilience;
unsigned char numberOfTemporalLayers;
@@ -434,6 +438,10 @@
// VP9 specific.
struct VideoCodecVP9 {
+ bool operator==(const VideoCodecVP9& other) const;
+ bool operator!=(const VideoCodecVP9& other) const {
+ return !(*this == other);
+ }
VideoCodecComplexity complexity;
bool resilienceOn;
unsigned char numberOfTemporalLayers;
@@ -461,6 +469,10 @@
// H264 specific.
struct VideoCodecH264 {
+ bool operator==(const VideoCodecH264& other) const;
+ bool operator!=(const VideoCodecH264& other) const {
+ return !(*this == other);
+ }
bool frameDroppingOn;
int keyFrameInterval;
// These are NULL/0 if not externally negotiated.
@@ -496,6 +508,9 @@
};
struct SpatialLayer {
+ bool operator==(const SpatialLayer& other) const;
+ bool operator!=(const SpatialLayer& other) const { return !(*this == other); }
+
unsigned short width;
unsigned short height;
unsigned char numberOfTemporalLayers;
diff --git a/modules/video_coding/encoder_database.cc b/modules/video_coding/encoder_database.cc
index 80716df..a514e14 100644
--- a/modules/video_coding/encoder_database.cc
+++ b/modules/video_coding/encoder_database.cc
@@ -138,23 +138,23 @@
switch (new_send_codec.codecType) {
case kVideoCodecVP8:
- if (memcmp(&new_send_codec.VP8(), send_codec_.VP8(),
- sizeof(new_send_codec.VP8())) != 0) {
+ if (new_send_codec.VP8() != *send_codec_.VP8()) {
return true;
}
break;
+
case kVideoCodecVP9:
- if (memcmp(&new_send_codec.VP9(), send_codec_.VP9(),
- sizeof(new_send_codec.VP9())) != 0) {
+ if (new_send_codec.VP9() != *send_codec_.VP9()) {
return true;
}
break;
+
case kVideoCodecH264:
- if (memcmp(&new_send_codec.H264(), send_codec_.H264(),
- sizeof(new_send_codec.H264())) != 0) {
+ if (new_send_codec.H264() != *send_codec_.H264()) {
return true;
}
break;
+
case kVideoCodecGeneric:
break;
// Known codecs without payload-specifics
@@ -169,15 +169,9 @@
return true;
}
- if (new_send_codec.numberOfSimulcastStreams > 0) {
- for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams;
- ++i) {
- if (memcmp(&new_send_codec.simulcastStream[i],
- &send_codec_.simulcastStream[i],
- sizeof(new_send_codec.simulcastStream[i])) != 0) {
- return true;
- }
- }
+ for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) {
+ if (new_send_codec.simulcastStream[i] != send_codec_.simulcastStream[i])
+ return true;
}
return false;
}