Fixing problems with RTP extension ID conflict resolution

talk/session/media/mediasession.cc

 /*
  * libjingle
  * Copyright 2004 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 889 matching lines @@
     if (it->uri == ext_to_match.uri) {
       if (found_extension != NULL) {
         *found_extension = *it;
       }
       return true;
     }
   }
   return false;
 }
 
-static void FindAndSetRtpHdrExtUsed(
-  const RtpHeaderExtensions& reference_extensions,
-  RtpHeaderExtensions* offered_extensions,
-  const RtpHeaderExtensions& other_extensions,
-  UsedRtpHeaderExtensionIds* used_extensions) {
-  for (RtpHeaderExtensions::const_iterator it = reference_extensions.begin();
-      it != reference_extensions.end(); ++it) {
-    if (!FindByUri(*offered_extensions, *it, NULL)) {
-      RtpHeaderExtension ext;
-      if (!FindByUri(other_extensions, *it, &ext)) {
-        ext = *it;
-        used_extensions->FindAndSetIdUsed(&ext);
-      }
-      offered_extensions->push_back(ext);
+// Iterates through |offered_extensions|, adding each one to |all_extensions|
+// and |used_ids|, and resolving ID conflicts. If an offered extension has the
+// same URI as one in |all_extensions|, it will re-use the same ID and won't be
+// treated as a conflict.
+static void FindAndSetRtpHdrExtUsed(RtpHeaderExtensions* offered_extensions,
+                                    RtpHeaderExtensions* all_extensions,
+                                    UsedRtpHeaderExtensionIds* used_ids) {
+  for (auto& extension : *offered_extensions) {
+    RtpHeaderExtension existing;
+    if (FindByUri(*all_extensions, extension, &existing)) {
+      extension.id = existing.id;
+    } else {
+      used_ids->FindAndSetIdUsed(&extension);
+      all_extensions->push_back(extension);
     }
   }
 }
+
+// Adds |reference_extensions| to |offered_extensions|, while updating
+// |all_extensions| and |used_ids|.
+static void FindRtpHdrExtsToOffer(
+    const RtpHeaderExtensions& reference_extensions,
+    RtpHeaderExtensions* offered_extensions,
+    RtpHeaderExtensions* all_extensions,
+    UsedRtpHeaderExtensionIds* used_ids) {
+  for (auto reference_extension : reference_extensions) {

[pthatcher1, 2015/08/18 20:33:12]

Can't it be "const auto&" instead of auto?

[Taylor Brandstetter, 2015/08/18 20:55:06]

No, because FindAndSetIdUsed modifies the id, and we don't want to modify the id
of our stored reference extensions.

+    if (!FindByUri(*offered_extensions, reference_extension, NULL)) {
+      RtpHeaderExtension existing;
+      if (FindByUri(*all_extensions, reference_extension, &existing)) {
+        offered_extensions->push_back(existing);
+      } else {
+        used_ids->FindAndSetIdUsed(&reference_extension);
+        all_extensions->push_back(reference_extension);
+        offered_extensions->push_back(reference_extension);
+      }
+    }
+  }
+}
 
 static void NegotiateRtpHeaderExtensions(
     const RtpHeaderExtensions& local_extensions,
     const RtpHeaderExtensions& offered_extensions,
     RtpHeaderExtensions* negotiated_extenstions) {
   RtpHeaderExtensions::const_iterator ours;
   for (ours = local_extensions.begin();
        ours != local_extensions.end(); ++ours) {
     RtpHeaderExtension theirs;
     if (FindByUri(offered_extensions, *ours, &theirs)) {
@@ skipping 454 matching lines @@
   FindCodecsToOffer<DataCodec>(data_codecs_, data_codecs, &used_pltypes);
 }
 
 void MediaSessionDescriptionFactory::GetRtpHdrExtsToOffer(
     const SessionDescription* current_description,
     RtpHeaderExtensions* audio_extensions,
     RtpHeaderExtensions* video_extensions) const {
   // All header extensions allocated from the same range to avoid potential
   // issues when using BUNDLE.
   UsedRtpHeaderExtensionIds used_ids;
+  RtpHeaderExtensions all_extensions;
   audio_extensions->clear();
   video_extensions->clear();
 
   // First - get all extensions from the current description if the media type
   // is used.
   // Add them to |used_ids| so the local ids are not reused if a new media
   // type is added.
   if (current_description) {
     const AudioContentDescription* audio =
         GetFirstAudioContentDescription(current_description);
     if (audio) {
       *audio_extensions = audio->rtp_header_extensions();
-      used_ids.FindAndSetIdUsed(audio_extensions);
+      FindAndSetRtpHdrExtUsed(audio_extensions, &all_extensions, &used_ids);
     }
     const VideoContentDescription* video =
         GetFirstVideoContentDescription(current_description);
     if (video) {
       *video_extensions = video->rtp_header_extensions();
-      used_ids.FindAndSetIdUsed(video_extensions);
+      FindAndSetRtpHdrExtUsed(video_extensions, &all_extensions, &used_ids);
     }
   }
 
   // Add our default RTP header extensions that are not in
   // |current_description|.
-  FindAndSetRtpHdrExtUsed(audio_rtp_header_extensions(), audio_extensions,
-                          *video_extensions, &used_ids);
-  FindAndSetRtpHdrExtUsed(video_rtp_header_extensions(), video_extensions,
-                          *audio_extensions, &used_ids);
+  FindRtpHdrExtsToOffer(audio_rtp_header_extensions(), audio_extensions,
+                        &all_extensions, &used_ids);
+  FindRtpHdrExtsToOffer(video_rtp_header_extensions(), video_extensions,
+                        &all_extensions, &used_ids);
 }
 
 bool MediaSessionDescriptionFactory::AddTransportOffer(
   const std::string& content_name,
   const TransportOptions& transport_options,
   const SessionDescription* current_desc,
   SessionDescription* offer_desc) const {
   if (!transport_desc_factory_)
      return false;
   const TransportDescription* current_tdesc =
@@ skipping 508 matching lines @@
   } else if (channel_name == GICE_CHANNEL_NAME_DATA_RTP ||
              channel_name == GICE_CHANNEL_NAME_DATA_RTCP) {
     *media_type = MEDIA_TYPE_DATA;
     return true;
   }
 
   return false;
 }
 
 }  // namespace cricket