Movable support for VideoReceiveStream::Config and avoid copies

webrtc/video/receive_statistics_proxy.h

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 19 matching lines @@
 class Clock;
 class ViECodec;
 class ViEDecoderObserver;
 struct CodecSpecificInfo;
 
 class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
                                public RtcpStatisticsCallback,
                                public RtcpPacketTypeCounterObserver,
                                public StreamDataCountersCallback {
  public:
-  ReceiveStatisticsProxy(const VideoReceiveStream::Config& config,
+  ReceiveStatisticsProxy(const VideoReceiveStream::Config* config,

[pbos-webrtc, 2016/06/06 16:15:34]

Shouldn't this still be a const reference?

[tommi, 2016/06/08 09:49:39]

It could.  What I want to do is to make a distinction between these two cases:

* how const& passing is used in general, which is that it's assumed that the
const& may be a temporary, a variable on the stack or a variable who's lifetime
does not have guarantees.
* pass a pointer to an object that will be shared between the caller and callee
for a known period of time.

The way the second case is usually implemented is by passing a non-const
pointer.  However, in our case here, it's still appropriate to have the object
const.

[pbos-webrtc, 2016/06/08 11:24:05]

I think we have quite a bit of video-related code that uses const references for
this, if we should start using pointers for these arguments I'd like for us to
do that as a cross-webrtc/video change instead of having this discrepancy. Does
that make sense?

E.g. in webrtc/video/ we usually handle the second case by having const
references, right now. I'm aboard with changing that, but we should do it across
the directory instead of having a style inconsistency.

[tommi, 2016/06/08 13:28:41]

I would prefer to keep this as is actually.  The semantics of how |config| and
|clock| are being passed, are comparable.

Furthermore, the other code is not following the style guide.  So I don't see a
reason for bulking up those sort of fixes when we can help with that work in
changelists like this one.

See: https://google.github.io/styleguide/cppguide.html#Reference_Arguments

                          Clock* clock);
   virtual ~ReceiveStatisticsProxy();
 
   VideoReceiveStream::Stats GetStats() const;
 
   void OnDecodedFrame();
   void OnSyncOffsetUpdated(int64_t sync_offset_ms);
   void OnRenderedFrame(int width, int height);
   void OnIncomingPayloadType(int payload_type);
   void OnDecoderImplementationName(const char* implementation_name);
@@ skipping 38 matching lines @@
     int sum;
     int num_samples;
   };
   struct QpCounters {
     SampleCounter vp8;
   };
 
   void UpdateHistograms() EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
   Clock* const clock_;
-  const VideoReceiveStream::Config config_;
+  // Ownership of this object lies with the owner of the ReceiveStatisticsProxy
+  // instance.  Lifetime is guaranteed to outlive |this|.
+  const VideoReceiveStream::Config& config_;
 
   rtc::CriticalSection crit_;
   VideoReceiveStream::Stats stats_ GUARDED_BY(crit_);
   RateStatistics decode_fps_estimator_ GUARDED_BY(crit_);
   RateStatistics renders_fps_estimator_ GUARDED_BY(crit_);
   rtc::RateTracker render_fps_tracker_ GUARDED_BY(crit_);
   rtc::RateTracker render_pixel_tracker_ GUARDED_BY(crit_);
   SampleCounter render_width_counter_ GUARDED_BY(crit_);
   SampleCounter render_height_counter_ GUARDED_BY(crit_);
   SampleCounter sync_offset_counter_ GUARDED_BY(crit_);
   SampleCounter decode_time_counter_ GUARDED_BY(crit_);
   SampleCounter jitter_buffer_delay_counter_ GUARDED_BY(crit_);
   SampleCounter target_delay_counter_ GUARDED_BY(crit_);
   SampleCounter current_delay_counter_ GUARDED_BY(crit_);
   SampleCounter delay_counter_ GUARDED_BY(crit_);
   ReportBlockStats report_block_stats_ GUARDED_BY(crit_);
   QpCounters qp_counters_;  // Only accessed on the decoding thread.
   std::map<uint32_t, StreamDataCounters> rtx_stats_ GUARDED_BY(crit_);
 };
 
 }  // namespace webrtc
 #endif  // WEBRTC_VIDEO_RECEIVE_STATISTICS_PROXY_H_