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}

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),

common_types.h

@@ -422,6 +422,10 @@ enum VP8ResilienceMode {
 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 @@ struct VideoCodecVP8 {
 
 // 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 @@ enum Profile {
 
 // 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 @@ union VideoCodecUnion {
 };
 
 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;

modules/video_coding/encoder_database.cc

@@ -138,23 +138,23 @@ bool VCMEncoderDataBase::RequiresEncoderReset(
 
   switch (new_send_codec.codecType) {
     case kVideoCodecVP8:
-      if (memcmp(&new_send_codec.VP8(), send_codec_.VP8(),
+      if (new_send_codec.VP8() != *send_codec_.VP8()) {
-                 sizeof(new_send_codec.VP8())) != 0) {
         return true;
       }
       break;
+
     case kVideoCodecVP9:
-      if (memcmp(&new_send_codec.VP9(), send_codec_.VP9(),
+      if (new_send_codec.VP9() != *send_codec_.VP9()) {
-                 sizeof(new_send_codec.VP9())) != 0) {
         return true;
       }
       break;
+
     case kVideoCodecH264:
-      if (memcmp(&new_send_codec.H264(), send_codec_.H264(),
+      if (new_send_codec.H264() != *send_codec_.H264()) {
-                 sizeof(new_send_codec.H264())) != 0) {
         return true;
       }
       break;
+
     case kVideoCodecGeneric:
       break;
     // Known codecs without payload-specifics
@@ -169,16 +169,10 @@ bool VCMEncoderDataBase::RequiresEncoderReset(
       return true;
   }
 
-  if (new_send_codec.numberOfSimulcastStreams > 0) {
+  for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) {
-    for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams;
+    if (new_send_codec.simulcastStream[i] != send_codec_.simulcastStream[i])
-         ++i) {
-      if (memcmp(&new_send_codec.simulcastStream[i],
-                 &send_codec_.simulcastStream[i],
-                 sizeof(new_send_codec.simulcastStream[i])) != 0) {
       return true;
   }
-    }
-  }
   return false;
 }