Delete all use of tick_util.h.

Delete all use of tick_util.h.

Depends on Chrome cl https://codereview.chromium.org/1888003002/, which was landed some time ago.

BUG=webrtc:5740
R=stefan@webrtc.org, tommi@webrtc.org

Committed: https://crrev.com/d28db7fd652bc1eb2cf7c10c948fb06cd9536398
Cr-Commit-Position: refs/heads/master@{#12674}

[nisse-webrtc, 2016-04-14 15:09:33]

nisse@webrtc.org changed reviewers:
+ stefan@webrtc.org, tommi@webrtc.org

[nisse-webrtc, 2016-04-14 15:09:43]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-14 15:09:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/1

[commit-bot: I haz the power, 2016-04-14 15:13:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-14 15:13:03]

Dry run: Try jobs failed on following builders:
  mac_asan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/13832)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/2309)

[stefan-webrtc, 2016-04-19 09:19:13]

Thanks for taking the time to clean this up.

Question for both you and tommi: Have we considered importing Chrome's
base/time/time.h instead of using webrtc/base/timeutils.h? Might be a better
choice if it's not too difficult importing.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
webrtc/audio/audio_receive_stream.cc:173: int64_t arrival_time_ms =
rtc::Time64();
Is there a function with a better name? I'd prefer if it actually said that the
time was in milliseconds.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/test/process_test.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/test/process_test.cc:692: int64_t tick_diff = t1
- t0;
tick_diff is a bad variable name now.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/test/process_test.cc:796: int64_t tick_diff = t1
- t0;
diff_nanos? Same below

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/mac/full_screen_chrome_window_detector.h
(right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/mac/full_screen_chrome_window_detector.h:61:
int64_t last_update_time_;
It'd be nice to add a unit to this member.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/screen_capturer_mac.mm (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/screen_capturer_mac.mm:384: int64_t
capture_start_time = rtc::TimeNanos();
capture_start_time_nanos

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/screen_capturer_x11.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/screen_capturer_x11.cc:237: int64_t
capture_start_time = rtc::TimeNanos();
_nanos

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/win/screen_capturer_win_gdi.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/win/screen_capturer_win_gdi.cc:98: int64_t
capture_start_time = rtc::TimeNanos();
_nanos

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc
(right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc:91: int64_t
capture_start_time = rtc::TimeNanos();
_nanos

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:119: // TickTime. TODO(nisse): Is
that now?
Not sure. We should remove it when the same clock implementation is used for
capture_time_ms (line 1117) as is used within rtp_rtcp.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/test/video_capture_unittest.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/test/video_capture_unittest.cc:496: int64_t
startTime = rtc::TimeNanos();
Seems like startTime should be the same type as returned by rtc::TimeNanos(),
and that way you'd avoid the casts below.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/video_capture_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/video_capture_impl.cc:97: _lastProcessTime =
rtc::TimeNanos();
Units

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/video_capture_impl.h (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/video_capture_impl.h:122: int64_t _lastProcessTime;
// last time the module process function was called.
Add units to these now that they aren't ticks.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/video_capture_impl.h:132: int64_t
_lastProcessFrameCount;
...Count is a weird name for a time

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/video_capture_impl.h:133: int64_t
_incomingFrameTimes[kFrameRateCountHistorySize];// timestamp for local captured
frames
unit

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/test/videoprocessor.h (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/test/videoprocessor.h:182: int64_t stop);
git cl format

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/test/videoprocessor.h:228: int64_t
decode_start_;
Units

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc:169: uint32_t seed =
rtc::Time32();
Not a big fan of the function name Time32 as it doesn't indicate if we're
talking minutes or milliseconds. The seed would be pretty bad if Time32 returns
the time in hours...

[tommi, 2016-04-19 09:24:07]

On 2016/04/19 09:19:13, stefan-webrtc (holmer) wrote:
> Thanks for taking the time to clean this up.
> 
> Question for both you and tommi: Have we considered importing Chrome's
> base/time/time.h instead of using webrtc/base/timeutils.h? Might be a better
> choice if it's not too difficult importing.

Yeah - Nisse, can you look into that?  There are some dependencies there, but
maybe we can fork it with making minimal changes.

[nisse-webrtc, 2016-04-19 12:18:52]

On 2016/04/19 09:24:07, tommi-webrtc wrote:
> On 2016/04/19 09:19:13, stefan-webrtc (holmer) wrote:
> > Thanks for taking the time to clean this up.
> > 
> > Question for both you and tommi: Have we considered importing Chrome's
> > base/time/time.h instead of using webrtc/base/timeutils.h? Might be a better
> > choice if it's not too difficult importing.
> 
> Yeah - Nisse, can you look into that?  There are some dependencies there, but
> maybe we can fork it with making minimal changes.

I've had a quick look at chrome's time.h, and I find it a bit too bloated for my
taste...
As far as I see, all we need here is an integer type, int64_t should be good
enough,
and some function to read the system's monotonic clock.

I agree it's desirable to make sure that webrtc and chrome have the same idea of
what the "system clock" is, and that it would be good to have some way to mock
the clock
so test cases don't need to call sleep. But it's unclear if copying code from
chrome moves
us in the right direction.

[nisse-webrtc, 2016-04-19 12:19:25]

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
webrtc/audio/audio_receive_stream.cc:173: int64_t arrival_time_ms =
rtc::Time64();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> Is there a function with a better name? I'd prefer if it actually said that
the
> time was in milliseconds.

There's another pending transition, I think. There's Time32 and Time64, which
both return milliseconds but with different size of the return value. And Time,
which currently is an alias for Time32, with a TODO to change it to Time64.

I could rename Time64 to TimeMs (or add an alias), but I'm not sure that's
appropriate for this cl.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/test/process_test.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/test/process_test.cc:692: int64_t tick_diff = t1
- t0;
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> tick_diff is a bad variable name now.

Renaming tick_diff --> diff_nanos and acc_ticks --> acc_nanos.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/mac/full_screen_chrome_window_detector.h
(right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/mac/full_screen_chrome_window_detector.h:61:
int64_t last_update_time_;
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> It'd be nice to add a unit to this member.

On a closer look, this member is unused. No explicit initialization (a bug in
the cl, the TickTime class had a default initializer), never assigned, and used
only to compute a most likely bogus interval for a warning message.

I'm deleting it instead.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/screen_capturer_mac.mm (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/screen_capturer_mac.mm:384: int64_t
capture_start_time = rtc::TimeNanos();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> capture_start_time_nanos

Done.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/screen_capturer_x11.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/screen_capturer_x11.cc:237: int64_t
capture_start_time = rtc::TimeNanos();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> _nanos

Done.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/win/screen_capturer_win_gdi.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/win/screen_capturer_win_gdi.cc:98: int64_t
capture_start_time = rtc::TimeNanos();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> _nanos

Done.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc
(right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc:91: int64_t
capture_start_time = rtc::TimeNanos();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> _nanos

Done.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:119: // TickTime. TODO(nisse): Is
that now?
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> Not sure. We should remove it when the same clock implementation is used for
> capture_time_ms (line 1117) as is used within rtp_rtcp. 

I expect more changes to timestamps. I'm dropping my added comment.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/test/video_capture_unittest.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/test/video_capture_unittest.cc:496: int64_t
startTime = rtc::TimeNanos();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> Seems like startTime should be the same type as returned by rtc::TimeNanos(),
> and that way you'd avoid the casts below.

Done. Typing seem a bit confused, TimeNanos and TimeMicros return unsigned
64-bit values, but signed int64_t is used for timestamps almost everywhere else.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/video_capture_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/video_capture_impl.cc:97: _lastProcessTime =
rtc::TimeNanos();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> Units

Done.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/video_capture_impl.h (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/video_capture_impl.h:122: int64_t _lastProcessTime;
// last time the module process function was called.
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> Add units to these now that they aren't ticks.

Adding "Nanos" to _lastProcessTime, _lastFrameRateCallbackTime, and
_incomingFrameTimes.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/video_capture_impl.h:132: int64_t
_lastProcessFrameCount;
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> ...Count is a weird name for a time

Renaming, to the somewhat awkward _lastProcessFrameTimeNanos.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/test/videoprocessor.h (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/test/videoprocessor.h:182: int64_t stop);
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> git cl format

Fixing this. Not doing a cl format of all the cl unless someone insists, it adds
a lot of uninteresting changes.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/test/videoprocessor.h:228: int64_t
decode_start_;
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> Units

Done.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc:169: uint32_t seed =
rtc::Time32();
On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> Not a big fan of the function name Time32 as it doesn't indicate if we're
> talking minutes or milliseconds. The seed would be pretty bad if Time32
returns
> the time in hours...

Do you have any concrete suggestion on what to do about it?

[tommi, 2016-04-19 12:28:16]

On 2016/04/19 12:18:52, nisse-webrtc wrote:
> On 2016/04/19 09:24:07, tommi-webrtc wrote:
> > On 2016/04/19 09:19:13, stefan-webrtc (holmer) wrote:
> > > Thanks for taking the time to clean this up.
> > > 
> > > Question for both you and tommi: Have we considered importing Chrome's
> > > base/time/time.h instead of using webrtc/base/timeutils.h? Might be a
better
> > > choice if it's not too difficult importing.
> > 
> > Yeah - Nisse, can you look into that?  There are some dependencies there,
but
> > maybe we can fork it with making minimal changes.
> 
> I've had a quick look at chrome's time.h, and I find it a bit too bloated for
my
> taste...
> As far as I see, all we need here is an integer type, int64_t should be good
> enough,
> and some function to read the system's monotonic clock.
> 
> I agree it's desirable to make sure that webrtc and chrome have the same idea
of
> what the "system clock" is, and that it would be good to have some way to mock
> the clock
> so test cases don't need to call sleep. But it's unclear if copying code from
> chrome moves
> us in the right direction.

Great, thanks for taking a look.  So let's not copy it over.

[stefan-webrtc, 2016-04-19 12:40:47]

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
webrtc/audio/audio_receive_stream.cc:173: int64_t arrival_time_ms =
rtc::Time64();
On 2016/04/19 12:19:25, nisse-webrtc wrote:
> On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> > Is there a function with a better name? I'd prefer if it actually said that
> the
> > time was in milliseconds.
> 
> There's another pending transition, I think. There's Time32 and Time64, which
> both return milliseconds but with different size of the return value. And
Time,
> which currently is an alias for Time32, with a TODO to change it to Time64.
> 
> I could rename Time64 to TimeMs (or add an alias), but I'm not sure that's
> appropriate for this cl.

I'd prefer an int64_t TimeMilliseconds() and maybe also a templated version of
it for the special cases where you want to put the time into for instance the
bitstream. But maybe the int64_t version is enough?

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/test/video_capture_unittest.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/test/video_capture_unittest.cc:496: int64_t
startTime = rtc::TimeNanos();
On 2016/04/19 12:19:25, nisse-webrtc wrote:
> On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> > Seems like startTime should be the same type as returned by
rtc::TimeNanos(),
> > and that way you'd avoid the casts below.
> 
> Done. Typing seem a bit confused, TimeNanos and TimeMicros return unsigned
> 64-bit values, but signed int64_t is used for timestamps almost everywhere
else.

Do you think there's a reason to return uint64_t from those methods, or should
we change to int64_t? Most code uses int64_t as it's using milliseconds mostly.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc:169: uint32_t seed =
rtc::Time32();
On 2016/04/19 12:19:25, nisse-webrtc wrote:
> On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> > Not a big fan of the function name Time32 as it doesn't indicate if we're
> > talking minutes or milliseconds. The seed would be pretty bad if Time32
> returns
> > the time in hours...
> 
> Do you have any concrete suggestion on what to do about it?

Maybe a templated method which returns time in milliseconds:
rtc::TimeMilliseconds<uint32_t>()

https://codereview.webrtc.org/1888593004/diff/20001/webrtc/modules/video_capt...
File webrtc/modules/video_capture/video_capture_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/20001/webrtc/modules/video_capt...
webrtc/modules/video_capture/video_capture_impl.cc:97: const int64_t now_ns =
rtc::TimeNanos();
Doesn't seem right that we store this in an int64_t if the method returns
uint64_t. Should we instead use milliseconds here? I think the same question is
valid for many other files you've changed.

[nisse-webrtc, 2016-04-19 13:48:51]

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/audio/audio_receive_st...
webrtc/audio/audio_receive_stream.cc:173: int64_t arrival_time_ms =
rtc::Time64();
On 2016/04/19 12:40:47, stefan-webrtc (holmer) wrote:
> On 2016/04/19 12:19:25, nisse-webrtc wrote:
> > On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> > > Is there a function with a better name? I'd prefer if it actually said
that
> > the
> > > time was in milliseconds.
> > 
> > There's another pending transition, I think. There's Time32 and Time64,
which
> > both return milliseconds but with different size of the return value. And
> Time,
> > which currently is an alias for Time32, with a TODO to change it to Time64.
> > 
> > I could rename Time64 to TimeMs (or add an alias), but I'm not sure that's
> > appropriate for this cl.
> 
> I'd prefer an int64_t TimeMilliseconds() and maybe also a templated version of
> it for the special cases where you want to put the time into for instance the
> bitstream. But maybe the int64_t version is enough?

I think int64_t is enough, no need for templates. I have to check if Time64 is
used anywhere, it's a bit silly with to have to keep that alias.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
File webrtc/modules/video_capture/test/video_capture_unittest.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_capture/...
webrtc/modules/video_capture/test/video_capture_unittest.cc:496: int64_t
startTime = rtc::TimeNanos();
On 2016/04/19 12:40:47, stefan-webrtc (holmer) wrote:
> On 2016/04/19 12:19:25, nisse-webrtc wrote:
> > On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> > > Seems like startTime should be the same type as returned by
> rtc::TimeNanos(),
> > > and that way you'd avoid the casts below.
> > 
> > Done. Typing seem a bit confused, TimeNanos and TimeMicros return unsigned
> > 64-bit values, but signed int64_t is used for timestamps almost everywhere
> else.
> 
> Do you think there's a reason to return uint64_t from those methods, or should
> we change to int64_t? Most code uses int64_t as it's using milliseconds
mostly.

I don't have a strong opinion here. Changing return type to int64_t (signed) is
fine with me. There seems to be a few places that assume that negative values
can't occur, though, using -1 to mean unset.

As far as I've seen, code using ns or us units also seem to use signed int64_t.

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc:169: uint32_t seed =
rtc::Time32();
On 2016/04/19 12:40:47, stefan-webrtc (holmer) wrote:
> On 2016/04/19 12:19:25, nisse-webrtc wrote:
> > On 2016/04/19 09:19:12, stefan-webrtc (holmer) wrote:
> > > Not a big fan of the function name Time32 as it doesn't indicate if we're
> > > talking minutes or milliseconds. The seed would be pretty bad if Time32
> > returns
> > > the time in hours...
> > 
> > Do you have any concrete suggestion on what to do about it?
> 
> Maybe a templated method which returns time in milliseconds:
> rtc::TimeMilliseconds<uint32_t>()

No templates just for this, please. Casting the return value to uint32_t should
be fine in the few places that really wants 32 bits.

https://codereview.webrtc.org/1888593004/diff/20001/webrtc/modules/video_capt...
File webrtc/modules/video_capture/video_capture_impl.cc (right):

https://codereview.webrtc.org/1888593004/diff/20001/webrtc/modules/video_capt...
webrtc/modules/video_capture/video_capture_impl.cc:97: const int64_t now_ns =
rtc::TimeNanos();
On 2016/04/19 12:40:47, stefan-webrtc (holmer) wrote:
> Doesn't seem right that we store this in an int64_t if the method returns
> uint64_t. Should we instead use milliseconds here? I think the same question
is
> valid for many other files you've changed.

I agree this isn't ideal, but it's no change: It's exactly what the TickTime
class was doing. This cl just removes the TickTime wrapper, it keeps ns units
everywhere, even if it's overkill.

[The Sun (google.com), 2016-04-19 17:46:40]

solenberg@google.com changed reviewers:
+ solenberg@google.com

[The Sun (google.com), 2016-04-19 17:46:41]

https://codereview.webrtc.org/1888593004/diff/60001/webrtc/system_wrappers/so...
File webrtc/system_wrappers/source/clock.cc (right):

https://codereview.webrtc.org/1888593004/diff/60001/webrtc/system_wrappers/so...
webrtc/system_wrappers/source/clock.cc:40: return rtc::Time64();
drive-by: Is the plan to change Time64()/Time32() into more meaningful names?
Sorry if this has been discussed already.

[nisse-webrtc, 2016-04-22 12:07:18]

On 2016/04/19 17:46:41, The Sun (google.com) wrote:
>
https://codereview.webrtc.org/1888593004/diff/60001/webrtc/system_wrappers/so...
> File webrtc/system_wrappers/source/clock.cc (right):
> 
>
https://codereview.webrtc.org/1888593004/diff/60001/webrtc/system_wrappers/so...
> webrtc/system_wrappers/source/clock.cc:40: return rtc::Time64();
> drive-by: Is the plan to change Time64()/Time32() into more meaningful names?
> Sorry if this has been discussed already.

I asked honghaiz@ about the naming. His response was that milliseconds is the
default time unit in webrtc, and that any renaming is better done in a separete
cl.

So, how to we move forward? I'd prefer to limit this cl to only the changes
which are necessary to delete tickutils. An alternative might to split it up
clean up one subsystem at the time, but that's going to be a much longer
process. 

Also, the dependency in chrome has been eliminated, when
https://codereview.chromium.org/1888003002/ landed a few days ago.

[stefan-webrtc, 2016-04-26 08:20:50]

On 2016/04/22 12:07:18, nisse-webrtc wrote:
> On 2016/04/19 17:46:41, The Sun (http://google.com) wrote:
> >
>
https://codereview.webrtc.org/1888593004/diff/60001/webrtc/system_wrappers/so...
> > File webrtc/system_wrappers/source/clock.cc (right):
> > 
> >
>
https://codereview.webrtc.org/1888593004/diff/60001/webrtc/system_wrappers/so...
> > webrtc/system_wrappers/source/clock.cc:40: return rtc::Time64();
> > drive-by: Is the plan to change Time64()/Time32() into more meaningful
names?
> > Sorry if this has been discussed already.
> 
> I asked honghaiz@ about the naming. His response was that milliseconds is the
> default time unit in webrtc, and that any renaming is better done in a
separete
> cl.
> 
> So, how to we move forward? I'd prefer to limit this cl to only the changes
> which are necessary to delete tickutils. An alternative might to split it up
> clean up one subsystem at the time, but that's going to be a much longer
> process. 
> 
> Also, the dependency in chrome has been eliminated, when
> https://codereview.chromium.org/1888003002/ landed a few days ago.

In that case I prefer landing the rename first, and then moving everything to
the new time functions. I don't like replacing TimeInMilliseconds() with
Time64() all over webrtc since it reduces readability quite a lot.

[nisse-webrtc, 2016-04-27 07:20:03]

> In that case I prefer landing the rename first, and then moving everything to
> the new time functions. I don't like replacing TimeInMilliseconds() with
> Time64() all over webrtc since it reduces readability quite a lot.

I see. What should the name be? "TimeInMillis" would be consistent with the
neighbor methods TimeInMicros and TimeInNanos. Who's going to have opinions on
the renaming? And is it possible to delete Time64 right away, or do we need to
keep it as an alias? I'll investigate, but if someone already knows the answers,
that's appreciated.

[stefan-webrtc, 2016-04-27 07:27:56]

On 2016/04/27 07:20:03, nisse-webrtc wrote:
> > In that case I prefer landing the rename first, and then moving everything
to
> > the new time functions. I don't like replacing TimeInMilliseconds() with
> > Time64() all over webrtc since it reduces readability quite a lot.
> 
> I see. What should the name be? "TimeInMillis" would be consistent with the
> neighbor methods TimeInMicros and TimeInNanos. Who's going to have opinions on
> the renaming? And is it possible to delete Time64 right away, or do we need to
> keep it as an alias? I'll investigate, but if someone already knows the
answers,
> that's appreciated.

TimeInMillis works for me. TimeInMilliseconds would also work, but having them
aligned is preferable. Who else would have an opinion? Probably everyone... :)

[pbos-webrtc, 2016-04-27 09:26:08]

On 2016/04/27 07:27:56, stefan-webrtc (holmer) wrote:
> On 2016/04/27 07:20:03, nisse-webrtc wrote:
> > > In that case I prefer landing the rename first, and then moving everything
> to
> > > the new time functions. I don't like replacing TimeInMilliseconds() with
> > > Time64() all over webrtc since it reduces readability quite a lot.
> > 
> > I see. What should the name be? "TimeInMillis" would be consistent with the
> > neighbor methods TimeInMicros and TimeInNanos. Who's going to have opinions
on
> > the renaming? And is it possible to delete Time64 right away, or do we need
to
> > keep it as an alias? I'll investigate, but if someone already knows the
> answers,
> > that's appreciated.
> 
> TimeInMillis works for me. TimeInMilliseconds would also work, but having them
> aligned is preferable. Who else would have an opinion? Probably everyone... :)

I'd prefer rtc::TimeMs() (and TimeUs()), but I'm also fine with red, green and
pink, so just paint. :)

[nisse-webrtc, 2016-05-02 15:40:12]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-02 15:40:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/80001

[commit-bot: I haz the power, 2016-05-02 15:46:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-02 15:46:50]

Dry run: Try jobs failed on following builders:
  android_clang_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...)

[nisse-webrtc, 2016-05-04 09:43:52]

Description was changed from

==========
Delete all use of tick_util.h.

Depends on Chrome cl https://codereview.chromium.org/1888003002/
before landing.

Not sure if the files are not deleted in this cl, or if there are
Chrome build files which have to be updated first.

BUG=webrtc:5740
==========

to

==========
Delete all use of tick_util.h.

Depends on Chrome cl https://codereview.chromium.org/1888003002/, which was
landed some time ago.

BUG=webrtc:5740
==========

[nisse-webrtc, 2016-05-04 10:52:10]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-04 10:52:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/120001

[commit-bot: I haz the power, 2016-05-04 11:09:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-04 11:09:44]

Dry run: Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/5292)

[stefan-webrtc, 2016-05-06 11:29:38]

lgtm

[nisse-webrtc, 2016-05-09 07:09:50]

Tommi, Solenberg, do you have any additional comments?

[tommi, 2016-05-09 07:48:39]

lgtm

[the sun, 2016-05-09 08:01:32]

On 2016/05/09 07:09:50, nisse-webrtc wrote:
> Tommi, Solenberg, do you have any additional comments?

Yes, sorry, didn't mean to stall this.
lgtm

[the sun, 2016-05-09 08:02:17]

solenberg@webrtc.org changed reviewers:
- solenberg@google.com

[nisse-webrtc, 2016-05-09 12:42:57]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-05-09 12:43:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/120001

[commit-bot: I haz the power, 2016-05-09 12:44:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-09 12:44:38]

Try jobs failed on following builders:
  android_compile_x64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x64_dbg...)
  android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...)
  android_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_dbg/builds/1...)
  android_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_rel/builds/1...)
  ios32_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/7311)
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/7298)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/9635)
  ios_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/9559)
  ios_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/14840)
  linux_asan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/14546)
  linux_libfuzzer_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...)
  linux_tsan2 on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/11996)
  linux_ubsan_vptr on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds...)
  mac_asan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/14464)
  mac_compile_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/2984)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/9207)
  mac_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/14947)
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/5382)

[nisse-webrtc, 2016-05-10 06:36:03]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-05-10 06:36:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, tommi@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1888593004/#ps140001 (title:
"Rebase.")

[commit-bot: I haz the power, 2016-05-10 06:36:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/140001

[commit-bot: I haz the power, 2016-05-10 06:47:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-10 06:47:07]

Try jobs failed on following builders:
  linux_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/14950)

[nisse-webrtc, 2016-05-10 08:39:41]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-05-10 08:39:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/140001

[commit-bot: I haz the power, 2016-05-10 08:50:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-10 08:50:41]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/5401)

[nisse-webrtc, 2016-05-10 13:11:49]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-05-10 13:11:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, tommi@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1888593004/#ps160001 (title:
"Break overlong lines.")

[commit-bot: I haz the power, 2016-05-10 13:12:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/160001

[commit-bot: I haz the power, 2016-05-10 13:19:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-10 13:19:59]

Try jobs failed on following builders:
  android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...)

[nisse-webrtc, 2016-05-10 13:26:55]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-05-10 13:27:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/160001

[commit-bot: I haz the power, 2016-05-10 13:34:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-10 13:34:02]

Try jobs failed on following builders:
  android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...)

[nisse-webrtc, 2016-05-10 13:49:38]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-05-10 13:49:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/160001

[commit-bot: I haz the power, 2016-05-10 13:55:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-10 13:55:33]

Try jobs failed on following builders:
  android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...)

[nisse-webrtc, 2016-05-10 14:11:42]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-05-10 14:11:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1888593004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1888593004/160001

[commit-bot: I haz the power, 2016-05-10 14:20:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-10 14:20:26]

Try jobs failed on following builders:
  android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...)

[nisse-webrtc, 2016-05-10 14:32:05]

Description was changed from

==========
Delete all use of tick_util.h.

Depends on Chrome cl https://codereview.chromium.org/1888003002/, which was
landed some time ago.

BUG=webrtc:5740
==========

to

==========
Delete all use of tick_util.h.

Depends on Chrome cl https://codereview.chromium.org/1888003002/, which was
landed some time ago.

BUG=webrtc:5740
R=stefan@webrtc.org, tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/d28db7fd652bc1eb2cf7c10c9...
==========

[commit-bot: I haz the power, 2016-05-10 14:32:06]

Description was changed from

==========
Delete all use of tick_util.h.

Depends on Chrome cl https://codereview.chromium.org/1888003002/, which was
landed some time ago.

BUG=webrtc:5740
R=stefan@webrtc.org, tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/d28db7fd652bc1eb2cf7c10c9...
==========

to

==========
Delete all use of tick_util.h.

Depends on Chrome cl https://codereview.chromium.org/1888003002/, which was
landed some time ago.

BUG=webrtc:5740
R=stefan@webrtc.org, tommi@webrtc.org

Committed: https://crrev.com/d28db7fd652bc1eb2cf7c10c948fb06cd9536398
Cr-Commit-Position: refs/heads/master@{#12674}
==========

[nisse-webrtc, 2016-05-10 14:32:08]

Committed patchset #10 (id:180001) manually as
d28db7fd652bc1eb2cf7c10c948fb06cd9536398 (presubmit successful).

[commit-bot: I haz the power, 2016-05-10 14:32:09]

Patchset 10 (id:??) landed as
https://crrev.com/d28db7fd652bc1eb2cf7c10c948fb06cd9536398
Cr-Commit-Position: refs/heads/master@{#12674}