Refactor RtpSender and SSRCDatabase a bit.

webrtc/base/random.h

Index: webrtc/base/random.h
diff --git a/webrtc/base/random.h b/webrtc/base/random.h
index 647b84c9c947b8d11f711012c9deb5e09b151d3f..bd9f4323b22e3aa561c7fae48cb95cd957d5598e 100644
--- a/webrtc/base/random.h
+++ b/webrtc/base/random.h
@@ -21,6 +21,14 @@ namespace webrtc {
 
 class Random {
  public:
+  // TODO(tommi): Change this so that the seed is initialized internally.
+  // At the moment all callers are calling clock_->TimeInMicroseconds() which
+  // goes through a global clock object (+vtable) and eventually calls
+  // TickTime::Now().Ticks(), which can return a very low value on Mac, which
+  // can result in a seed of 0 after conversion to microseconds.
+  // Instead see if we can use something from webrtc/base that works the same
+  // way across platforms (and is itself more random than the Mac implementation
+  // of TickTime::Now()).

[the sun, 2016/01/24 21:34:10]

Maybe, but the ctor taking a seed is still needed for tests. We want (unit)
tests to be reproducible, not randomized (fuzzers are a different matter of
course). That, and the issue with global state was the reason to create this
class and not use std::rand().

Also, depending on what you actually mean we should use to init the seed, it
doesn't ring right to me adding a dependency on TickTime (or Clock) to a PRNG.
The PRNG is a platform-independent number crunching utility. Anything we can
keep platform independent, we should.

Suggest instead:
a) Add a TickTime::Get64BitForPRNGSeed() utility function which can be used with
whichever PRNG class/impl we have now or in the future.
b) If that is too inconvenient, invent a template function which hooks up above
utility to a PRNG class.

[tommi, 2016/01/24 21:54:35]

Maybe we can do both?  If Random calls an overridable function to get the seed,
tests could use that.  Today all callers provide a seed from the same source.

btw, could you point me to a bug or something related to using this
implementation instead of std::rand?  I'm not following why this class couldn't
use it internally at least.
To be clear, this depends on TickTime today indirectly (via Clock) and there's a
problem with that on Mac due to how TickTime::Now() is implemented.

I don't think I'm following the comment about platform independence though.
Sounds like you're reading the comment as being about making the code platform
dependent, which is not the point.  TickTime is itself cross platform interface
but has platform specific implementations.  So too would Get64BitForPRNGSeed be,
I imagine?
How would it be implemented?

[stefan-webrtc, 2016/01/25 10:09:32]

I think what solenberg is saying is that it would be better to have a static
Get64BitForPRNGSeed() in TickTime (using TickTime::Now().Ticks()) than in
Random. I can live with both.

I agree with solenberg that we still want to be able to pass a seed to this
constructor in the future for tests.

[the sun, 2016/01/25 11:24:57]

No, tests use constant seeds. I can spot three uses of Random outside of unit
tests. Requiring the tests to override a seed function sounds like more
code+work than adding the one call to fetch a seed in rtp_sender, rtcp_sender
and ssrc_database.
Random was originally created for the BWE simulation framework, since
std::rand():
1) is platform specific (potentially different test sequences on different
platforms)
2) uses global state (scheduling impacts actual test sequence)

It has since been picked up and used in other places, mainly in tests. Recently,
terelius@ refined the interface and implementation. Personally I think it's very
nice not needing to do "std::rand() % range" everywhere.
Platform independence (in this context) == "take the source files and feed them
to a C++ compiler without having to muck with compiler settings" - a.k.a.
portable C++. So in my mind, Random is portable, but TickTime is not (it
requires the right compiler+flags+include paths), even if the interface
certainly is.

In any case, adding a dependency from Random to TickTime, makes Random less
encapsulated and less reusable. I would rather add that link outside of either
module.
template<typename PRNG> PRNG InitPrng() {
  return PRNG(TickTime::Get64BitForPRNGSeed());
}

InitPrng<Random>()

(assumes state is copyable of course)

[tommi, 2016/01/25 12:28:33]

Great, I think I get it now :)  I'll take a closer look at this at some later
point (or someone else can too of course).

Note that Random can't depend on TickCount as I point out in the comment, but I
think I get where you're coming from.  Sounds like you agree with the part about
"works the same way across platforms" too.

   explicit Random(uint64_t seed);
 
   // Return pseudo-random integer of the specified type.