Bug 1080470 - [RTSP] Avoid unnecessary play/pause requests to RTSP server in RspControllerChild. r=sworkman
authorEthan Tseng <ettseng@mozilla.com>
Thu, 09 Oct 2014 18:44:50 +0800
changeset 210139 b0d430fb34b75666cfbd0d370a6b13cd9c0cc65d
parent 210138 f634aef97d4709ea39d76a786e014191f57837c2
child 210140 2dcd05eca248911603b579b6affe23ee6177f312
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerssworkman
bugs1080470
milestone35.0a1
Bug 1080470 - [RTSP] Avoid unnecessary play/pause requests to RTSP server in RspControllerChild. r=sworkman
netwerk/protocol/rtsp/controller/RtspController.cpp
netwerk/protocol/rtsp/controller/RtspController.h
netwerk/protocol/rtsp/controller/RtspControllerChild.cpp
netwerk/protocol/rtsp/controller/RtspControllerChild.h
--- a/netwerk/protocol/rtsp/controller/RtspController.cpp
+++ b/netwerk/protocol/rtsp/controller/RtspController.cpp
@@ -42,32 +42,27 @@
 #include "zlib.h"
 #include <algorithm>
 #include "nsDebug.h"
 
 extern PRLogModuleInfo* gRtspLog;
 #undef LOG
 #define LOG(args) PR_LOG(gRtspLog, PR_LOG_DEBUG, args)
 
-const unsigned long kCommandDelayMs = 200;
-
 namespace mozilla {
 namespace net {
 
 //-----------------------------------------------------------------------------
 // RtspController
 //-----------------------------------------------------------------------------
 NS_IMPL_ISUPPORTS(RtspController,
                   nsIStreamingProtocolController)
 
 RtspController::RtspController(nsIChannel *channel)
-  : mState(INIT),
-    mTimerLock("RtspController.mTimerLock"),
-    mPlayTimer(nullptr),
-    mPauseTimer(nullptr)
+  : mState(INIT)
 {
   LOG(("RtspController::RtspController()"));
 }
 
 RtspController::~RtspController()
 {
   LOG(("RtspController::~RtspController()"));
   if (mRtspSource.get()) {
@@ -94,72 +89,34 @@ RtspController::Play(void)
     MOZ_ASSERT(mRtspSource.get(), "mRtspSource should not be null!");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   if (mState != CONNECTED) {
     return NS_ERROR_NOT_CONNECTED;
   }
 
-  MutexAutoLock lock(mTimerLock);
-  // Cancel the pause timer if it is active because successive pause-play in a
-  // short duration is unnecessary but could impair playback smoothing.
-  if (mPauseTimer) {
-    mPauseTimer->Cancel();
-    mPauseTimer = nullptr;
-  }
-
-  // Start a timer to delay the play operation for a short duration.
-  if (!mPlayTimer) {
-    mPlayTimer = do_CreateInstance("@mozilla.org/timer;1");
-    if (!mPlayTimer) {
-      return NS_ERROR_NOT_INITIALIZED;
-    }
-    mPlayTimer->InitWithFuncCallback(
-                  RtspController::PlayTimerCallback,
-                  this, kCommandDelayMs,
-                  nsITimer::TYPE_ONE_SHOT);
-  }
-
+  mRtspSource->play();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RtspController::Pause(void)
 {
   LOG(("RtspController::Pause()"));
   if (!mRtspSource.get()) {
     MOZ_ASSERT(mRtspSource.get(), "mRtspSource should not be null!");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   if (mState != CONNECTED) {
     return NS_ERROR_NOT_CONNECTED;
   }
 
-  MutexAutoLock lock(mTimerLock);
-  // Cancel the play timer if it is active because successive play-pause in a
-  // short duration is unnecessary but could impair playback smoothing.
-  if (mPlayTimer) {
-    mPlayTimer->Cancel();
-    mPlayTimer = nullptr;
-  }
-
-  // Start a timer to delay the pause operation for a short duration.
-  if (!mPauseTimer) {
-    mPauseTimer = do_CreateInstance("@mozilla.org/timer;1");
-    if (!mPauseTimer) {
-      return NS_ERROR_NOT_INITIALIZED;
-    }
-    mPauseTimer->InitWithFuncCallback(
-                   RtspController::PauseTimerCallback,
-                   this, kCommandDelayMs,
-                   nsITimer::TYPE_ONE_SHOT);
-  }
-
+  mRtspSource->pause();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RtspController::Resume(void)
 {
   return Play();
 }
@@ -355,29 +312,16 @@ private:
 
 NS_IMETHODIMP
 RtspController::OnDisconnected(uint8_t index,
                                nsresult reason)
 {
   LOG(("RtspController::OnDisconnected() for track %d reason = 0x%x", index, reason));
   mState = DISCONNECTED;
 
-  // Ensure play and pause timer are stopped.
-  {
-    MutexAutoLock lock(mTimerLock);
-    if (mPlayTimer) {
-      mPlayTimer->Cancel();
-      mPlayTimer = nullptr;
-    }
-    if (mPauseTimer) {
-      mPauseTimer->Cancel();
-      mPauseTimer = nullptr;
-    }
-  }
-
   if (mListener) {
     nsRefPtr<SendOnDisconnectedTask> task =
       new SendOnDisconnectedTask(mListener, index, reason);
     // Break the cycle reference between the Listener (RtspControllerParent) and
     // us.
     mListener = nullptr;
     return NS_DispatchToMainThread(task);
   }
@@ -431,45 +375,10 @@ RtspController::PlaybackEnded()
     MOZ_ASSERT(mRtspSource.get(), "mRtspSource should not be null!");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   mRtspSource->playbackEnded();
   return NS_OK;
 }
 
-//-----------------------------------------------------------------------------
-// RtspController static member methods
-//-----------------------------------------------------------------------------
-//static
-void RtspController::PlayTimerCallback(nsITimer *aTimer, void *aClosure)
-{
-  MOZ_ASSERT(aTimer);
-  MOZ_ASSERT(aClosure);
-
-  RtspController *self = static_cast<RtspController*>(aClosure);
-  MOZ_ASSERT(self->mRtspSource.get());
-
-  MutexAutoLock lock(self->mTimerLock);
-  if (self->mPlayTimer) {
-    self->mRtspSource->play();
-    self->mPlayTimer = nullptr;
-  }
-}
-
-//static
-void RtspController::PauseTimerCallback(nsITimer *aTimer, void *aClosure)
-{
-  MOZ_ASSERT(aTimer);
-  MOZ_ASSERT(aClosure);
-
-  RtspController *self = static_cast<RtspController*>(aClosure);
-  MOZ_ASSERT(self->mRtspSource.get());
-
-  MutexAutoLock lock(self->mTimerLock);
-  if (self->mPauseTimer) {
-    self->mRtspSource->pause();
-    self->mPauseTimer = nullptr;
-  }
-}
-
 } // namespace mozilla::net
 } // namespace mozilla
--- a/netwerk/protocol/rtsp/controller/RtspController.h
+++ b/netwerk/protocol/rtsp/controller/RtspController.h
@@ -2,42 +2,36 @@
 /* vim: set sw=2 ts=8 et tw=80 : */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef RtspController_h
 #define RtspController_h
 
-#include "mozilla/Mutex.h"
 #include "nsIStreamingProtocolController.h"
 #include "nsIChannel.h"
 #include "nsCOMPtr.h"
 #include "nsString.h"
-#include "nsITimer.h"
 #include "RTSPSource.h"
 
 namespace mozilla {
 namespace net {
 
 class RtspController : public nsIStreamingProtocolController
                      , public nsIStreamingProtocolListener
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSISTREAMINGPROTOCOLCONTROLLER
   NS_DECL_NSISTREAMINGPROTOCOLLISTENER
 
   RtspController(nsIChannel *channel);
   ~RtspController();
 
-  // These callbacks will be called when mPlayTimer/mPauseTimer fires.
-  static void PlayTimerCallback(nsITimer *aTimer, void *aClosure);
-  static void PauseTimerCallback(nsITimer *aTimer, void *aClosure);
-
 private:
   enum State {
     INIT,
     CONNECTED,
     DISCONNECTED
   };
 
   // RTSP URL refer to a stream or an aggregate of streams.
@@ -48,20 +42,13 @@ private:
   nsCString mSpec;
   // UserAgent string.
   nsCString mUserAgent;
   // Indicate the connection state between the
   // media streaming server and the Rtsp client.
   State mState;
   // Rtsp Streaming source.
   android::sp<android::RTSPSource> mRtspSource;
-  // This lock protects mPlayTimer and mPauseTimer.
-  Mutex mTimerLock;
-  // Timers to delay the play and pause operations.
-  // They are used for optimization and avoid sending unnecessary requests to
-  // the server.
-  nsCOMPtr<nsITimer> mPlayTimer;
-  nsCOMPtr<nsITimer> mPauseTimer;
 };
 
 }
 } // namespace mozilla::net
 #endif
--- a/netwerk/protocol/rtsp/controller/RtspControllerChild.cpp
+++ b/netwerk/protocol/rtsp/controller/RtspControllerChild.cpp
@@ -16,16 +16,18 @@
 #include "nsStringStream.h"
 #include "prlog.h"
 
 PRLogModuleInfo* gRtspChildLog = nullptr;
 #undef LOG
 #define LOG(args) PR_LOG(gRtspChildLog, PR_LOG_DEBUG, args)
 
 const uint32_t kRtspTotalTracks = 2;
+const unsigned long kRtspCommandDelayMs = 200;
+
 using namespace mozilla::ipc;
 
 namespace mozilla {
 namespace net {
 
 NS_IMPL_ADDREF(RtspControllerChild)
 
 NS_IMETHODIMP_(nsrefcnt) RtspControllerChild::Release()
@@ -59,16 +61,19 @@ NS_INTERFACE_MAP_END
 // RtspControllerChild methods
 //-----------------------------------------------------------------------------
 RtspControllerChild::RtspControllerChild(nsIChannel *channel)
   : mIPCOpen(false)
   , mIPCAllowed(false)
   , mChannel(channel)
   , mTotalTracks(0)
   , mSuspendCount(0)
+  , mTimerLock("RtspControllerChild.mTimerLock")
+  , mPlayTimer(nullptr)
+  , mPauseTimer(nullptr)
 {
 #if defined(PR_LOGGING)
   if (!gRtspChildLog)
     gRtspChildLog = PR_NewLogModule("nsRtspChild");
 #endif
   AddIPDLReference();
   gNeckoChild->SendPRtspControllerConstructor(this);
 }
@@ -103,16 +108,30 @@ RtspControllerChild::AllowIPC()
 
 void
 RtspControllerChild::DisallowIPC()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mIPCAllowed = false;
 }
 
+void
+RtspControllerChild::StopPlayAndPauseTimer()
+{
+  MutexAutoLock lock(mTimerLock);
+  if (mPlayTimer) {
+    mPlayTimer->Cancel();
+    mPlayTimer = nullptr;
+  }
+  if (mPauseTimer) {
+    mPauseTimer->Cancel();
+    mPauseTimer = nullptr;
+  }
+}
+
 //-----------------------------------------------------------------------------
 // RtspControllerChild::PRtspControllerChild
 //-----------------------------------------------------------------------------
 bool
 RtspControllerChild::RecvOnMediaDataAvailable(
                        const uint8_t& index,
                        const nsCString& data,
                        const uint32_t& length,
@@ -169,28 +188,30 @@ RtspControllerChild::RecvOnConnected(
   return true;
 }
 
 bool
 RtspControllerChild::RecvOnDisconnected(
                        const uint8_t& index,
                        const nsresult& reason)
 {
+  StopPlayAndPauseTimer();
   DisallowIPC();
   LOG(("RtspControllerChild::RecvOnDisconnected for track %d reason = 0x%x", index, reason));
   if (mListener) {
     mListener->OnDisconnected(index, reason);
   }
   ReleaseChannel();
   return true;
 }
 
 bool
 RtspControllerChild::RecvAsyncOpenFailed(const nsresult& reason)
 {
+  StopPlayAndPauseTimer();
   DisallowIPC();
   LOG(("RtspControllerChild::RecvAsyncOpenFailed reason = 0x%x", reason));
   if (mListener) {
     mListener->OnDisconnected(0, NS_ERROR_CONNECTION_REFUSED);
   }
   ReleaseChannel();
   return true;
 }
@@ -229,18 +250,16 @@ RtspControllerChild::GetTrackMetaData(
 }
 
 enum IPCEvent
 {
   SendNoneEvent = 0,
   SendPlayEvent,
   SendPauseEvent,
   SendSeekEvent,
-  SendResumeEvent,
-  SendSuspendEvent,
   SendStopEvent,
   SendPlaybackEndedEvent
 };
 
 class SendIPCEvent : public nsRunnable
 {
 public:
   SendIPCEvent(RtspControllerChild *aController, IPCEvent aEvent)
@@ -270,20 +289,16 @@ public:
     bool rv = true;
 
     if (mEvent == SendPlayEvent) {
       rv = mController->SendPlay();
     } else if (mEvent == SendPauseEvent) {
       rv = mController->SendPause();
     } else if (mEvent == SendSeekEvent) {
       rv = mController->SendSeek(mSeekTime);
-    } else if (mEvent == SendResumeEvent) {
-      rv = mController->SendResume();
-    } else if (mEvent == SendSuspendEvent) {
-      rv = mController->SendSuspend();
     } else if (mEvent == SendStopEvent) {
       rv = mController->SendStop();
     } else if (mEvent == SendPlaybackEndedEvent) {
       rv = mController->SendPlaybackEnded();
     } else {
       LOG(("RtspControllerChild::SendIPCEvent"));
     }
     if (!rv) {
@@ -300,83 +315,98 @@ private:
 //-----------------------------------------------------------------------------
 // RtspControllerChild::nsIStreamingProtocolController
 //-----------------------------------------------------------------------------
 NS_IMETHODIMP
 RtspControllerChild::Play(void)
 {
   LOG(("RtspControllerChild::Play()"));
 
-  if (NS_IsMainThread()) {
-    if (!OKToSendIPC() || !SendPlay()) {
-      return NS_ERROR_FAILURE;
+  MutexAutoLock lock(mTimerLock);
+  // Cancel the pause timer if it is active because successive pause-play in a
+  // short duration is unncessary but could impair playback smoothing.
+  if (mPauseTimer) {
+    mPauseTimer->Cancel();
+    mPauseTimer = nullptr;
+  }
+
+  // Start a timer to delay the play operation for a short duration.
+  if (!mPlayTimer) {
+    mPlayTimer = do_CreateInstance("@mozilla.org/timer;1");
+    if (!mPlayTimer) {
+      return NS_ERROR_NOT_INITIALIZED;
     }
-  } else {
-    nsresult rv = NS_DispatchToMainThread(
-                    new SendIPCEvent(this, SendPlayEvent));
-    NS_ENSURE_SUCCESS(rv, rv);
+    // We have to dispatch the timer callback to the main thread because the
+    // decoder thread is a thread from nsIThreadPool and cannot be the timer
+    // target. Furthermore, IPC send functions should only be called from the
+    // main thread.
+    nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+    mPlayTimer->SetTarget(mainThread);
+    mPlayTimer->InitWithFuncCallback(
+                  RtspControllerChild::PlayTimerCallback,
+                  this, kRtspCommandDelayMs,
+                  nsITimer::TYPE_ONE_SHOT);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RtspControllerChild::Pause(void)
 {
   LOG(("RtspControllerChild::Pause()"));
 
-  if (NS_IsMainThread()) {
-    if (!OKToSendIPC() || !SendPause()) {
-      return NS_ERROR_FAILURE;
+  MutexAutoLock lock(mTimerLock);
+  // Cancel the play timer if it is active because successive play-pause in a
+  // shrot duration is unnecessary but could impair playback smoothing.
+  if (mPlayTimer) {
+    mPlayTimer->Cancel();
+    mPlayTimer = nullptr;
+  }
+
+  // Start a timer to delay the pause operation for a short duration.
+  if (!mPauseTimer) {
+    mPauseTimer = do_CreateInstance("@mozilla.org/timer;1");
+    if (!mPauseTimer) {
+      return NS_ERROR_NOT_INITIALIZED;
     }
-  } else {
-    nsresult rv = NS_DispatchToMainThread(
-                    new SendIPCEvent(this, SendPauseEvent));
-    NS_ENSURE_SUCCESS(rv, rv);
+    // We have to dispatch the timer callback to the main thread because the
+    // decoder thread is a thread from nsIThreadPool and cannot be the timer
+    // target.
+    nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+    mPauseTimer->SetTarget(mainThread);
+    mPauseTimer->InitWithFuncCallback(
+                  RtspControllerChild::PauseTimerCallback,
+                  this, kRtspCommandDelayMs,
+                  nsITimer::TYPE_ONE_SHOT);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RtspControllerChild::Resume(void)
 {
   LOG(("RtspControllerChild::Resume()"));
   NS_ENSURE_TRUE(mSuspendCount > 0, NS_ERROR_UNEXPECTED);
 
   if (!--mSuspendCount) {
-    if (NS_IsMainThread()) {
-      if (!OKToSendIPC() || !SendResume()) {
-        return NS_ERROR_FAILURE;
-      }
-    } else {
-      nsresult rv = NS_DispatchToMainThread(
-                      new SendIPCEvent(this, SendResumeEvent));
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
+    return Play();
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RtspControllerChild::Suspend(void)
 {
   LOG(("RtspControllerChild::Suspend()"));
 
   if (!mSuspendCount++) {
-    if (NS_IsMainThread()) {
-      if (!OKToSendIPC() || !SendSuspend()) {
-        return NS_ERROR_FAILURE;
-      }
-    } else {
-      nsresult rv = NS_DispatchToMainThread(
-                      new SendIPCEvent(this, SendSuspendEvent));
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
+    return Pause();
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RtspControllerChild::Seek(uint64_t seekTimeUs)
 {
@@ -394,16 +424,17 @@ RtspControllerChild::Seek(uint64_t seekT
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RtspControllerChild::Stop()
 {
   LOG(("RtspControllerChild::Stop()"));
+  StopPlayAndPauseTimer();
 
   if (NS_IsMainThread()) {
     if (!OKToSendIPC() || !SendStop()) {
       return NS_ERROR_FAILURE;
     }
     DisallowIPC();
   } else {
     nsresult rv = NS_DispatchToMainThread(
@@ -537,10 +568,49 @@ RtspControllerChild::AsyncOpen(nsIStream
   SerializeURI(uri, uriParams);
 
   if (!OKToSendIPC() || !SendAsyncOpen(uriParams)) {
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
+//-----------------------------------------------------------------------------
+// RtspControllerChild static member methods
+//-----------------------------------------------------------------------------
+//static
+void
+RtspControllerChild::PlayTimerCallback(nsITimer *aTimer, void *aClosure)
+{
+  MOZ_ASSERT(aTimer);
+  MOZ_ASSERT(aClosure);
+  MOZ_ASSERT(NS_IsMainThread());
+
+  RtspControllerChild *self = static_cast<RtspControllerChild*>(aClosure);
+
+  MutexAutoLock lock(self->mTimerLock);
+  if (!self->mPlayTimer || !self->OKToSendIPC()) {
+    return;
+  }
+  self->SendPlay();
+  self->mPlayTimer = nullptr;
+}
+
+//static
+void
+RtspControllerChild::PauseTimerCallback(nsITimer *aTimer, void *aClosure)
+{
+  MOZ_ASSERT(aTimer);
+  MOZ_ASSERT(aClosure);
+  MOZ_ASSERT(NS_IsMainThread());
+
+  RtspControllerChild *self = static_cast<RtspControllerChild*>(aClosure);
+
+  MutexAutoLock lock(self->mTimerLock);
+  if (!self->mPauseTimer || !self->OKToSendIPC()) {
+    return;
+  }
+  self->SendPause();
+  self->mPauseTimer = nullptr;
+}
+
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/rtsp/controller/RtspControllerChild.h
+++ b/netwerk/protocol/rtsp/controller/RtspControllerChild.h
@@ -9,16 +9,18 @@
 
 #include "mozilla/net/PRtspControllerChild.h"
 #include "nsIStreamingProtocolController.h"
 #include "nsIChannel.h"
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "mozilla/net/RtspChannelChild.h"
+#include "mozilla/Mutex.h"
+#include "nsITimer.h"
 
 namespace mozilla {
 namespace net {
 
 class RtspControllerChild : public nsIStreamingProtocolController
                           , public nsIStreamingProtocolListener
                           , public PRtspControllerChild
 {
@@ -47,16 +49,20 @@ class RtspControllerChild : public nsISt
   void AddIPDLReference();
   void ReleaseIPDLReference();
   void AddMetaData(already_AddRefed<nsIStreamingProtocolMetaData>&& meta);
   int  GetMetaDataLength();
   bool OKToSendIPC();
   void AllowIPC();
   void DisallowIPC();
 
+  // These callbacks will be called when mPlayTimer/mPauseTimer fires.
+  static void PlayTimerCallback(nsITimer *aTimer, void *aClosure);
+  static void PauseTimerCallback(nsITimer *aTimer, void *aClosure);
+
  private:
   bool mIPCOpen;
   // The intention of this variable is just to avoid any IPC message to be sent
   // when this flag is set as false. Nothing more.
   bool mIPCAllowed;
   // Dummy channel used to aid MediaResource creation in HTMLMediaElement.
   nsCOMPtr<nsIChannel> mChannel;
   // The nsIStreamingProtocolListener implementation.
@@ -68,13 +74,23 @@ class RtspControllerChild : public nsISt
   // ASCII encoded URL spec
   nsCString mSpec;
   // The total tracks for the given media stream session.
   uint32_t mTotalTracks;
   // Current suspension depth for this channel object
   uint32_t mSuspendCount;
   // Detach channel-controller relationship.
   void ReleaseChannel();
+  // This lock protects mPlayTimer and mPauseTimer.
+  Mutex mTimerLock;
+  // Timers to delay the play and pause operations.
+  // They are used for optimization and to avoid sending unnecessary requests to
+  // the server.
+  nsCOMPtr<nsITimer> mPlayTimer;
+  nsCOMPtr<nsITimer> mPauseTimer;
+  // Timers should be stopped if we are going to terminate, such as when
+  // receiving Stop command or OnDisconnected event.
+  void StopPlayAndPauseTimer();
 };
 } // namespace net
 } // namespace mozilla
 
 #endif // RtspControllerChild_h