Reverse |rtx_payload_types| map, and rename.

webrtc/video_receive_stream.h

Index: webrtc/video_receive_stream.h
diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h
index 48d16a8ea1b7f883d8652deceb694512c0946cfa..b9649736551df64cd59f30d7d9aabcd4f9a93cee 100644
--- a/webrtc/video_receive_stream.h
+++ b/webrtc/video_receive_stream.h
@@ -161,9 +161,9 @@ class VideoReceiveStream {
       // Set if the stream is protected using FlexFEC.
       bool protected_by_flexfec = false;
 
-      // Map from video payload type (apt) -> RTX payload type (pt).
+      // Map from rtx payload type -> media payload type.
       // For RTX to be enabled, both an SSRC and this mapping are needed.
-      std::map<int, int> rtx_payload_types;
+      std::map<int, int> media_pt_by_rtx_pt;

[danilchap, 2017/08/21 14:11:26]

non-common abbreviations in names are strongly discouraged
https://google.github.io/styleguide/cppguide.html#General_Naming_Rules

May be media_by_rtx_payload_type

[nisse-webrtc, 2017/08/22 07:11:01]

My intention was to make the name so explicit that we don't need any comment to
say what the keys and values represent, and then |media_by...| isn't quite good
enough. (And the comment should perhaps be improved, delete the part about keys
and values, and instead explain what's the purpose of the mapping).

I could change to |media_payload_type_by_rtx_payload_type|, but if that is a bit
too unwieldy, I think abbreviating payload_type in the name is clearer than
deleting it. 

And I'd consider "pt" is a reasonably common abbreviation in webrtc, with 178
matches for grep -iw pt...

Suggestions?

[danilchap, 2017/08/22 08:09:55]

I still find 'pt' both 'Only your group knows what this stands for' and
'Ambiguous abbreviation': If you try to search what pt means, you'll find 5
different meanings, none of them is 'payload type'
It might be fine to use it in very small scope, but it is a bad word for large
scope.

Another option is to unuse map to have more descriptive names instead of
first/second instead of putting both of them into variable name, e.g.:

struct RtxBinding {
  int retransmission_payload_type;
  int associated_payload_type;
  // may be later add int rtx_time_ms;
};
std::vector<RtxBinding> rtx_bindings_;

// using names from section 8.1 of the
https://tools.ietf.org/html/rfc4588#page-20

 
       // RTP header extensions used for the received stream.
       std::vector<RtpExtension> extensions;