Refactor: simplify interface of SenderRtcpSession and CastTransport.

media/cast/sender/frame_sender.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // This is the base class for an object that send frames to a receiver.
 // TODO(hclam): Refactor such that there is no separate AudioSender vs.
 // VideoSender, and the functionality of both is rolled into this class.
 
 #ifndef MEDIA_CAST_SENDER_FRAME_SENDER_H_
 #define MEDIA_CAST_SENDER_FRAME_SENDER_H_
@@ skipping 35 matching lines @@
   void SetTargetPlayoutDelay(base::TimeDelta new_target_playout_delay);
 
   base::TimeDelta GetTargetPlayoutDelay() const {
     return target_playout_delay_;
   }
 
   // Called by the encoder with the next EncodeFrame to send.
   void SendEncodedFrame(int requested_bitrate_before_encode,
                         scoped_ptr<SenderEncodedFrame> encoded_frame);
 
+  // Protected for testability.
+  void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback);

[miu, 2016/04/15 23:14:39]

Instead of making these methods public, consider one of two options:

1. Have FrameSender extend and implement the RtpSenderRtcpClient interface
directly (i.e., no separate class).

-or-

2. You should give the RtpSenderRtcpClient subclass access to the protected
members of FrameSender.  This can be done by forward-declaring
FrameSenderRtcpClient as an inner class of FrameSender.  Meaning:

  class FrameSender {
    ...
   protected:
    class RtcpClient;
    ...
  };

Then, in frame_sender.cc:

class FrameSender::RtcpClient : public RtpSenderRtcpClient {
  ...code in this class has access to protected and private members of
FrameSender...
};

(And, note that you only need one of these; and not a separate one for
AudioSender vs VideoSender.)

IMO, #1 would be much simpler than #2.  Perhaps the same approach would work for
other classes involved in this change?

[xjz, 2016/04/20 01:09:03]

Done. Choose #2 for convenience.

+
+  // Called when a Pli message is received.
+  void OnReceivedPli();
+
+  void OnMeasuredRoundTripTime(base::TimeDelta rtt);
+
  protected:
   // Returns the number of frames in the encoder's backlog.
   virtual int GetNumberOfFramesInEncoder() const = 0;
 
   // Returns the duration of the data in the encoder's backlog plus the duration
   // of sent, unacknowledged frames.
   virtual base::TimeDelta GetInFlightMediaDuration() const = 0;
 
  protected:
   // Schedule and execute periodic sending of RTCP report.
   void ScheduleNextRtcpReport();
   void SendRtcpReport(bool schedule_future_reports);
 
-  void OnMeasuredRoundTripTime(base::TimeDelta rtt);
-
   const scoped_refptr<CastEnvironment> cast_environment_;
 
   // Sends encoded frames over the configured transport (e.g., UDP).  In
   // Chromium, this could be a proxy that first sends the frames from a renderer
   // process to the browser process over IPC, with the browser process being
   // responsible for "packetizing" the frames and pushing packets into the
   // network layer.
   CastTransport* const transport_sender_;
 
   const uint32_t ssrc_;
 
  protected:
   // Schedule and execute periodic checks for re-sending packets.  If no
   // acknowledgements have been received for "too long," AudioSender will
   // speculatively re-send certain packets of an unacked frame to kick-start
   // re-transmission.  This is a last resort tactic to prevent the session from
   // getting stuck after a long outage.
   void ScheduleNextResendCheck();
   void ResendCheck();
   void ResendForKickstart();
 
-  // Protected for testability.
-  void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback);
-
-  // Called when a Pli message is received.
-  void OnReceivedPli();
-
   // Returns true if too many frames would be in-flight by encoding and sending
   // the next frame having the given |frame_duration|.
   bool ShouldDropNextFrame(base::TimeDelta frame_duration) const;
 
   // Record or retrieve a recent history of each frame's timestamps.
   // Warning: If a frame ID too far in the past is requested, the getters will
   // silently succeed but return incorrect values.  Be sure to respect
   // media::cast::kMaxUnackedFrames.
   void RecordLatestFrameTimestamps(uint32_t frame_id,
                                    base::TimeTicks reference_time,
@@ skipping 84 matching lines @@
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<FrameSender> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FrameSender);
 };
 
 }  // namespace cast
 }  // namespace media
 
 #endif  // MEDIA_CAST_SENDER_FRAME_SENDER_H_