Refactor RtpSender and SSRCDatabase a bit.

webrtc/modules/rtp_rtcp/source/ssrc_database.cc

 /*
  *  Copyright (c) 2011 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.
  */
 
 #include "webrtc/modules/rtp_rtcp/source/ssrc_database.h"
 
 #include "webrtc/base/checks.h"
-#include "webrtc/system_wrappers/include/clock.h"
-#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
+#include "webrtc/system_wrappers/include/tick_util.h"
 
 namespace webrtc {
-namespace {
-uint64_t Seed() {
-  return Clock::GetRealTimeClock()->TimeInMicroseconds();
-}
-}  // namespace
 
 SSRCDatabase* SSRCDatabase::GetSSRCDatabase() {
   return GetStaticInstance<SSRCDatabase>(kAddRef);
 }
 
 void SSRCDatabase::ReturnSSRCDatabase() {
   GetStaticInstance<SSRCDatabase>(kRelease);
 }
 
+// TODO(tommi, holmer): Look into whether we can eliminate locking in this
+// class.  At the moment voe_auto_test requires it, but it's not clear if that's
+// an issue with the test code or if it reflects real world usage or if that's
+// the best design performance wise.

[stefan-webrtc, 2016/01/26 13:19:52]

Should this mention getting rid of the static instance, and in that case, should
it mention removing this class entirely once voice engine no longer relies on
this being a static instance?

[tommi, 2016/01/30 10:53:41]

Comment updated and moved to the header.

 uint32_t SSRCDatabase::CreateSSRC() {
-  CriticalSectionScoped lock(crit_.get());
+  rtc::CritScope lock(&crit_);
 
   while (true) {  // Try until get a new ssrc.
     // 0 and 0xffffffff are invalid values for SSRC.
     uint32_t ssrc = random_.Rand(1u, 0xfffffffe);
     if (ssrcs_.insert(ssrc).second) {
       return ssrc;
     }
   }
 }
 
 void SSRCDatabase::RegisterSSRC(uint32_t ssrc) {
-  CriticalSectionScoped lock(crit_.get());
+  rtc::CritScope lock(&crit_);
   ssrcs_.insert(ssrc);
 }
 
 void SSRCDatabase::ReturnSSRC(uint32_t ssrc) {
-  CriticalSectionScoped lock(crit_.get());
+  rtc::CritScope lock(&crit_);
   ssrcs_.erase(ssrc);
 }
 
-SSRCDatabase::SSRCDatabase()
-    : crit_(CriticalSectionWrapper::CreateCriticalSection()), random_(Seed()) {}
+// For initializing the random seed, we don't use TimeInMicroseconds() since it
+// can return 0 on Mac if we're  called early on during process startup (e.g.
+// in tests).  This is because when the tick count gets converted into
+// microseconds, we can end up multiplying with 0 and end up with a seed of 0.
+// TODO(tommi): Move this into the Random() class itself.  At the moment that's
+// not possible because the Random class is in webrtc/base but TickTime is
+// in webrtc/system_wrappers/include/.

[stefan-webrtc, 2016/01/26 13:19:52]

As mentioned in earlier comments, an option would be to move this to
TickTime::GenerateRandomSeed() so that we can write:
random_(GenerateRandomSeed())

I can live with this being resolved later though.

[tommi, 2016/01/30 10:53:41]

Updated comment.  Erik has been making some improvements to timing code in
webrtc/base so I think we should be able to add something like a
GenerateRandomSeed() function there.

+SSRCDatabase::SSRCDatabase() : random_(TickTime::Now().Ticks()) {}
 
-SSRCDatabase::~SSRCDatabase() {
-}
+SSRCDatabase::~SSRCDatabase() {}
 
 }  // namespace webrtc