Added VP8 simulcast tests. Fixed analyzer to correctly infer timestamps.

Added VP8 simulcast tests. Fixed analyzer to correctly infer timestamps.


BUG=webrtc:7095
Review-Url: https://codereview.webrtc.org/2668763004
Cr-Commit-Position: refs/heads/master@{#16428}
Committed: https://chromium.googlesource.com/external/webrtc/+/5f4712686550ba1d069c5e4c456ffcabe7ccba97

[ilnik, 2017-02-01 12:16:38]

ilnik@webrtc.org changed reviewers:
+ sprang@webrtc.org

[ilnik, 2017-02-01 12:34:28]

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

[commit-bot: I haz the power, 2017-02-01 12:34:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-01 12:39:06]

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

[commit-bot: I haz the power, 2017-02-01 12:39:07]

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

[ilnik, 2017-02-01 12:44:43]

Description was changed from

==========
Added VP8 simulcast tests. Fixed analyzer to correctly infer timestamps.


BUG=
==========

to

==========
Added VP8 simulcast tests. Fixed analyzer to correctly infer timestamps.


BUG=webrtc:7095
==========

[ilnik, 2017-02-01 12:50:18]

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

[commit-bot: I haz the power, 2017-02-01 12:50:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-01 14:03:23]

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

[commit-bot: I haz the power, 2017-02-01 14:03:24]

Dry run: This issue passed the CQ dry run.

[ilnik, 2017-02-01 14:25:28]

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

[commit-bot: I haz the power, 2017-02-01 14:25:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[sprang_webrtc, 2017-02-01 14:50:20]

Looks good!
To automatically comply with style guide re formatting, try running "git cl
format".
+nisse, do you have an opinion re timestamp on EncodedFrame?

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/common_video/inclu...
File webrtc/common_video/include/frame_callback.h (right):

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/common_video/inclu...
webrtc/common_video/include/frame_callback.h:25: EncodedFrame() : data_(NULL),
length_(0), frame_type_(kEmptyFrame),
nit: nullptr

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/common_video/inclu...
webrtc/common_video/include/frame_callback.h:39: const int32_t timestamp_;
Hm, constructor parameter is unsigned but member is signed?
Not sure which kind of timestamp this is, but I think we're moving towards using
ns timestamps with int64_t type wherever we can, so might be a good idea here
too.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
File webrtc/video/full_stack_tests.cc (right):

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:356: simulcast.pipe.loss_percent = 1;
I'm thinking we should start out without packet loss and stress that in upcoming
tests instead. WDYT?

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:358: // Needs high enough bandwidth for
high-bitrate stream to be not dropped
nit: End comments with period.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:359: simulcast.pipe.link_capacity_kbps = 3000;
Or even don't set this so it's unrestricted?

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:365: video_params_medium.video = {true, 896,
504, 50, 60000, 500000, 500000,
Please try matching the default settings for simulcast.
Normally, a 3sl case will use 1280x720, 640x360, 320x180 combo.
See webrtc/media/engine/simulcast.cc for a table of default bitrates etc.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
File webrtc/video/video_quality_test.cc (right):

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
webrtc/video/video_quality_test.cc:237: // Also ignore packets from wrong SSRC
nit: End comments with period.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
webrtc/video/video_quality_test.cc:272: first_sent_timestamp_ =
rtc::Optional<uint32_t>(encoded_frame.timestamp_);
You can also do
  first_sent_timestamp_.emplace(encoded_frame.timestamp_);

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
webrtc/video/video_quality_test.cc:296: // Also ignore packets from wrong SSRC
dito

[ilnik, 2017-02-01 15:12:50]

Fixed all your comments

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/common_video/inclu...
File webrtc/common_video/include/frame_callback.h (right):

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/common_video/inclu...
webrtc/common_video/include/frame_callback.h:25: EncodedFrame() : data_(NULL),
length_(0), frame_type_(kEmptyFrame),
On 2017/02/01 14:50:19, språng wrote:
> nit: nullptr

Done.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/common_video/inclu...
webrtc/common_video/include/frame_callback.h:39: const int32_t timestamp_;
On 2017/02/01 14:50:19, språng wrote:
> Hm, constructor parameter is unsigned but member is signed?
> Not sure which kind of timestamp this is, but I think we're moving towards
using
> ns timestamps with int64_t type wherever we can, so might be a good idea here
> too.

Done.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
File webrtc/video/full_stack_tests.cc (right):

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:356: simulcast.pipe.loss_percent = 1;
On 2017/02/01 14:50:19, språng wrote:
> I'm thinking we should start out without packet loss and stress that in
upcoming
> tests instead. WDYT?

Done.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:358: // Needs high enough bandwidth for
high-bitrate stream to be not dropped
On 2017/02/01 14:50:20, språng wrote:
> nit: End comments with period.

Done.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:359: simulcast.pipe.link_capacity_kbps = 3000;
On 2017/02/01 14:50:19, språng wrote:
> Or even don't set this so it's unrestricted?

Great idea. Will do that.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/full_stack_t...
webrtc/video/full_stack_tests.cc:365: video_params_medium.video = {true, 896,
504, 50, 60000, 500000, 500000,
On 2017/02/01 14:50:19, språng wrote:
> Please try matching the default settings for simulcast.
> Normally, a 3sl case will use 1280x720, 640x360, 320x180 combo.
> See webrtc/media/engine/simulcast.cc for a table of default bitrates etc.

Done.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
File webrtc/video/video_quality_test.cc (right):

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
webrtc/video/video_quality_test.cc:237: // Also ignore packets from wrong SSRC
On 2017/02/01 14:50:20, språng wrote:
> nit: End comments with period.

Done.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
webrtc/video/video_quality_test.cc:272: first_sent_timestamp_ =
rtc::Optional<uint32_t>(encoded_frame.timestamp_);
On 2017/02/01 14:50:20, språng wrote:
> You can also do
>   first_sent_timestamp_.emplace(encoded_frame.timestamp_);

Acknowledged.

https://codereview.webrtc.org/2668763004/diff/40001/webrtc/video/video_qualit...
webrtc/video/video_quality_test.cc:296: // Also ignore packets from wrong SSRC
On 2017/02/01 14:50:20, språng wrote:
> dito

Done.

[sprang_webrtc, 2017-02-01 15:33:47]

sprang@webrtc.org changed reviewers:
+ nisse@webrtc.org

[sprang_webrtc, 2017-02-01 15:33:48]

lgtm
and actually added nisse this time..

[ilnik, 2017-02-01 15:36:58]

ilnik@webrtc.org changed reviewers:
+ mflodman@webrtc.org

[nisse-webrtc, 2017-02-01 15:55:43]

On 2017/02/01 14:50:20, språng wrote:
> Looks good!
> To automatically comply with style guide re formatting, try running "git cl
> format".
> +nisse, do you have an opinion re timestamp on EncodedFrame?

I've had a quick look. It seems the timestamp is only used for test and the
PostEncodeFrameCallback? It's undesirable to add fields to "production" structs
for the sole benefit of tests. So it would be nice if it could be passed in some
other way in the test code.

That said, our various structs and classes for reprepresenting encoded frames
are a mess, and most of them do include an RTP 90 kHz timestamp.

Maybe you could instead delete this type, and pass an EncodedImage instead
(declared in webrtc/VideoFrame.h)? That struct includes width, height, and a
couple of different timestamps. I think that's the type which is used at the
call sites of EncodedFrameCallback.

[ilnik, 2017-02-01 16:04:57]

On 2017/02/01 15:55:43, nisse-webrtc wrote:
> On 2017/02/01 14:50:20, språng wrote:
> > Looks good!
> > To automatically comply with style guide re formatting, try running "git cl
> > format".
> > +nisse, do you have an opinion re timestamp on EncodedFrame?
> 
> I've had a quick look. It seems the timestamp is only used for test and the
> PostEncodeFrameCallback? It's undesirable to add fields to "production"
structs
> for the sole benefit of tests. So it would be nice if it could be passed in
some
> other way in the test code.
> 
> That said, our various structs and classes for reprepresenting encoded frames
> are a mess, and most of them do include an RTP 90 kHz timestamp.
> 
> Maybe you could instead delete this type, and pass an EncodedImage instead
> (declared in webrtc/VideoFrame.h)? That struct includes width, height, and a
> couple of different timestamps. I think that's the type which is used at the
> call sites of EncodedFrameCallback.

This EncodedFrame struct is used only in tests in EncodedFrameCallback. This
entire struct can be dropped as it's a subset of EncodedImage and is always
derived from it. Do you think I should just remove it?

[nisse-webrtc, 2017-02-01 16:22:12]

nisse@webrtc.org changed reviewers:
+ magjed@webrtc.org

[nisse-webrtc, 2017-02-01 16:25:15]

On 2017/02/01 16:04:57, ilnik wrote:
> On 2017/02/01 15:55:43, nisse-webrtc wrote:
> > On 2017/02/01 14:50:20, språng wrote:
> > > Looks good!
> > > To automatically comply with style guide re formatting, try running "git
cl
> > > format".
> > > +nisse, do you have an opinion re timestamp on EncodedFrame?
> > 
> > I've had a quick look. It seems the timestamp is only used for test and the
> > PostEncodeFrameCallback? It's undesirable to add fields to "production"
> structs
> > for the sole benefit of tests. So it would be nice if it could be passed in
> some
> > other way in the test code.
> > 
> > That said, our various structs and classes for reprepresenting encoded
frames
> > are a mess, and most of them do include an RTP 90 kHz timestamp.
> > 
> > Maybe you could instead delete this type, and pass an EncodedImage instead
> > (declared in webrtc/VideoFrame.h)? That struct includes width, height, and a
> > couple of different timestamps. I think that's the type which is used at the
> > call sites of EncodedFrameCallback.
> 
> This EncodedFrame struct is used only in tests in EncodedFrameCallback. This
> entire struct can be dropped as it's a subset of EncodedImage and is always
> derived from it. Do you think I should just remove it?

Deletion makes sense to me. Let's check with a common_video owner. magjed, what
do you think?

[magjed_webrtc, 2017-02-01 17:20:57]

On 2017/02/01 16:25:15, nisse-webrtc wrote:
> On 2017/02/01 16:04:57, ilnik wrote:
> > On 2017/02/01 15:55:43, nisse-webrtc wrote:
> > > On 2017/02/01 14:50:20, språng wrote:
> > > > Looks good!
> > > > To automatically comply with style guide re formatting, try running "git
> cl
> > > > format".
> > > > +nisse, do you have an opinion re timestamp on EncodedFrame?
> > > 
> > > I've had a quick look. It seems the timestamp is only used for test and
the
> > > PostEncodeFrameCallback? It's undesirable to add fields to "production"
> > structs
> > > for the sole benefit of tests. So it would be nice if it could be passed
in
> > some
> > > other way in the test code.
> > > 
> > > That said, our various structs and classes for reprepresenting encoded
> frames
> > > are a mess, and most of them do include an RTP 90 kHz timestamp.
> > > 
> > > Maybe you could instead delete this type, and pass an EncodedImage instead
> > > (declared in webrtc/VideoFrame.h)? That struct includes width, height, and
a
> > > couple of different timestamps. I think that's the type which is used at
the
> > > call sites of EncodedFrameCallback.
> > 
> > This EncodedFrame struct is used only in tests in EncodedFrameCallback. This
> > entire struct can be dropped as it's a subset of EncodedImage and is always
> > derived from it. Do you think I should just remove it?
> 
> Deletion makes sense to me. Let's check with a common_video owner. magjed,
what
> do you think?

If it's not used externally, delete as much you can. Try removing it and then
run a chromium.fyi bot as well as 'git cl try -m
master.internal.tryserver.corp.webrtc -b linux_internal'. If those passes, then
go ahead with the removal.

[nisse-webrtc, 2017-02-02 08:17:05]

On 2017/02/01 17:20:57, magjed_webrtc wrote:
> On 2017/02/01 16:25:15, nisse-webrtc wrote:
> > On 2017/02/01 16:04:57, ilnik wrote:
> > > On 2017/02/01 15:55:43, nisse-webrtc wrote:
> > > > On 2017/02/01 14:50:20, språng wrote:
> > > > > Looks good!
> > > > > To automatically comply with style guide re formatting, try running
"git
> > cl
> > > > > format".
> > > > > +nisse, do you have an opinion re timestamp on EncodedFrame?
> > > > 
> > > > I've had a quick look. It seems the timestamp is only used for test and
> the
> > > > PostEncodeFrameCallback? It's undesirable to add fields to "production"
> > > structs
> > > > for the sole benefit of tests. So it would be nice if it could be passed
> in
> > > some
> > > > other way in the test code.
> > > > 
> > > > That said, our various structs and classes for reprepresenting encoded
> > frames
> > > > are a mess, and most of them do include an RTP 90 kHz timestamp.
> > > > 
> > > > Maybe you could instead delete this type, and pass an EncodedImage
instead
> > > > (declared in webrtc/VideoFrame.h)? That struct includes width, height,
and
> a
> > > > couple of different timestamps. I think that's the type which is used at
> the
> > > > call sites of EncodedFrameCallback.
> > > 
> > > This EncodedFrame struct is used only in tests in EncodedFrameCallback.
This
> > > entire struct can be dropped as it's a subset of EncodedImage and is
always
> > > derived from it. Do you think I should just remove it?
> > 
> > Deletion makes sense to me. Let's check with a common_video owner. magjed,
> what
> > do you think?
> 
> If it's not used externally, delete as much you can. Try removing it and then
> run a chromium.fyi bot as well as 'git cl try -m
> master.internal.tryserver.corp.webrtc -b linux_internal'. If those passes,
then
> go ahead with the removal.

I'll leave the strategy up to you (ilnik@), but it might simplify the process to
do the deletion in a separate cl and land that first, and next rebase this one
on top. Then we'd know if deletion worked out before updating this cl.

[ilnik, 2017-02-02 08:53:12]

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

[commit-bot: I haz the power, 2017-02-02 08:53:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[ilnik, 2017-02-02 08:58:51]

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

[commit-bot: I haz the power, 2017-02-02 08:58:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[sprang_webrtc, 2017-02-02 09:07:03]

On 2017/02/02 08:17:05, nisse-webrtc wrote:
> On 2017/02/01 17:20:57, magjed_webrtc wrote:
> > On 2017/02/01 16:25:15, nisse-webrtc wrote:
> > > On 2017/02/01 16:04:57, ilnik wrote:
> > > > On 2017/02/01 15:55:43, nisse-webrtc wrote:
> > > > > On 2017/02/01 14:50:20, språng wrote:
> > > > > > Looks good!
> > > > > > To automatically comply with style guide re formatting, try running
> "git
> > > cl
> > > > > > format".
> > > > > > +nisse, do you have an opinion re timestamp on EncodedFrame?
> > > > > 
> > > > > I've had a quick look. It seems the timestamp is only used for test
and
> > the
> > > > > PostEncodeFrameCallback? It's undesirable to add fields to
"production"
> > > > structs
> > > > > for the sole benefit of tests. So it would be nice if it could be
passed
> > in
> > > > some
> > > > > other way in the test code.
> > > > > 
> > > > > That said, our various structs and classes for reprepresenting encoded
> > > frames
> > > > > are a mess, and most of them do include an RTP 90 kHz timestamp.
> > > > > 
> > > > > Maybe you could instead delete this type, and pass an EncodedImage
> instead
> > > > > (declared in webrtc/VideoFrame.h)? That struct includes width, height,
> and
> > a
> > > > > couple of different timestamps. I think that's the type which is used
at
> > the
> > > > > call sites of EncodedFrameCallback.
> > > > 
> > > > This EncodedFrame struct is used only in tests in EncodedFrameCallback.
> This
> > > > entire struct can be dropped as it's a subset of EncodedImage and is
> always
> > > > derived from it. Do you think I should just remove it?
> > > 
> > > Deletion makes sense to me. Let's check with a common_video owner. magjed,
> > what
> > > do you think?
> > 
> > If it's not used externally, delete as much you can. Try removing it and
then
> > run a chromium.fyi bot as well as 'git cl try -m
> > master.internal.tryserver.corp.webrtc -b linux_internal'. If those passes,
> then
> > go ahead with the removal.
> 
> I'll leave the strategy up to you (ilnik@), but it might simplify the process
to
> do the deletion in a separate cl and land that first, and next rebase this one
> on top. Then we'd know if deletion worked out before updating this cl.

I think the purpose of EncodedFrame was to not expose all of the legacy things
in EncodedImage when the "new api" was created. I think we at least need to
notify people if we intend to remove EncodedFrame.
I also see there's a todo in EncodedImage to clean it up and rename it to
EncodedFrame.

[ilnik, 2017-02-02 09:17:21]

The CQ bit was unchecked by ilnik@webrtc.org

[ilnik, 2017-02-02 09:17:22]

The CQ bit was checked by ilnik@webrtc.org

[commit-bot: I haz the power, 2017-02-02 09:17:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-02 09:21:25]

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

[commit-bot: I haz the power, 2017-02-02 09:21:26]

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

[nisse-webrtc, 2017-02-02 09:21:35]

On 2017/02/02 09:07:03, språng wrote:
> I think the purpose of EncodedFrame was to not expose all of the legacy things
> in EncodedImage when the "new api" was created.

New as in 2013... So that seems to have been a stalled effort?

> I think we at least need to
> notify people if we intend to remove EncodedFrame.
> I also see there's a todo in EncodedImage to clean it up and rename it to
> EncodedFrame.

EncodedImage and its half dozen subclasses is a mess that desparately needs
cleanup. But it's going to be a little delicate, due to external encoders and
decoders depending on webrtc/video_video_{en,de}coder.h. To make progress, we
probably need to assemble a little cabal offline to forge plans. And we're going
to need one or two timestamps also after cleaning it up.

[ilnik, 2017-02-02 09:29:38]

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

[commit-bot: I haz the power, 2017-02-02 09:29:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[ilnik, 2017-02-02 09:51:19]

On 2017/02/02 09:21:35, nisse-webrtc wrote:
> On 2017/02/02 09:07:03, språng wrote:
> > I think the purpose of EncodedFrame was to not expose all of the legacy
things
> > in EncodedImage when the "new api" was created.
> 
> New as in 2013... So that seems to have been a stalled effort?
> 
> > I think we at least need to
> > notify people if we intend to remove EncodedFrame.
> > I also see there's a todo in EncodedImage to clean it up and rename it to
> > EncodedFrame.
> 
> EncodedImage and its half dozen subclasses is a mess that desparately needs
> cleanup. But it's going to be a little delicate, due to external encoders and
> decoders depending on webrtc/video_video_{en,de}coder.h. To make progress, we
> probably need to assemble a little cabal offline to forge plans. And we're
going
> to need one or two timestamps also after cleaning it up.

So, I will commit the current Cl, since EncodedFrame is needed but did not
expose timestamps because no one thought it would be used.
I will leave refactoring of EncodedFrame/EncodedImage for later. 

@magjed_webrtc, please give me LGTM. I will run chromium.fyi trybots once I gain
access there (somehow, I can't run jobs there yet). After that I will land the
changes.

[commit-bot: I haz the power, 2017-02-02 09:52:10]

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

[commit-bot: I haz the power, 2017-02-02 09:52:11]

Dry run: This issue passed the CQ dry run.

[ilnik, 2017-02-02 15:31:34]

On 2017/02/02 09:51:19, ilnik wrote:
> On 2017/02/02 09:21:35, nisse-webrtc wrote:
> > On 2017/02/02 09:07:03, språng wrote:
> > > I think the purpose of EncodedFrame was to not expose all of the legacy
> things
> > > in EncodedImage when the "new api" was created.
> > 
> > New as in 2013... So that seems to have been a stalled effort?
> > 
> > > I think we at least need to
> > > notify people if we intend to remove EncodedFrame.
> > > I also see there's a todo in EncodedImage to clean it up and rename it to
> > > EncodedFrame.
> > 
> > EncodedImage and its half dozen subclasses is a mess that desparately needs
> > cleanup. But it's going to be a little delicate, due to external encoders
and
> > decoders depending on webrtc/video_video_{en,de}coder.h. To make progress,
we
> > probably need to assemble a little cabal offline to forge plans. And we're
> going
> > to need one or two timestamps also after cleaning it up.
> 
> So, I will commit the current Cl, since EncodedFrame is needed but did not
> expose timestamps because no one thought it would be used.
> I will leave refactoring of EncodedFrame/EncodedImage for later. 
> 
> @magjed_webrtc, please give me LGTM. I will run chromium.fyi trybots once I
gain
> access there (somehow, I can't run jobs there yet). After that I will land the
> changes.

All trybots (including chromium) passed. I also checked google3 and chromium CS
- this structure is not used anywhere outside of our internal tests.
I still need LGTM for /common_video/include/frame_callback.h

[magjed_webrtc, 2017-02-02 15:57:15]

lgtm

[ilnik, 2017-02-03 09:10:14]

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

[commit-bot: I haz the power, 2017-02-03 09:10:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-03 09:49:20]

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

[commit-bot: I haz the power, 2017-02-03 09:49:21]

Dry run: This issue passed the CQ dry run.

[ilnik, 2017-02-03 09:59:47]

The CQ bit was checked by ilnik@webrtc.org

[ilnik, 2017-02-03 09:59:47]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
sprang@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2668763004/#ps80001 (title:
"reapplyting patch to clean branch")

[commit-bot: I haz the power, 2017-02-03 10:00:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-03 10:02:18]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486115987473310,
"parent_rev": "4b512d7e0c1d342f50b47c304bbcdd06f6564754", "commit_rev":
"5f4712686550ba1d069c5e4c456ffcabe7ccba97"}

[commit-bot: I haz the power, 2017-02-03 10:02:22]

Description was changed from

==========
Added VP8 simulcast tests. Fixed analyzer to correctly infer timestamps.


BUG=webrtc:7095
==========

to

==========
Added VP8 simulcast tests. Fixed analyzer to correctly infer timestamps.


BUG=webrtc:7095

Review-Url: https://codereview.webrtc.org/2668763004
Cr-Commit-Position: refs/heads/master@{#16428}
Committed:
https://chromium.googlesource.com/external/webrtc/+/5f4712686550ba1d069c5e4c4...
==========

[commit-bot: I haz the power, 2017-02-03 10:02:24]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/5f4712686550ba1d069c5e4c4...

[ilnik, 2017-02-09 09:20:18]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.webrtc.org/2687073002/ by ilnik@webrtc.org.

The reason for reverting is: Speculative revert due to regression in perf
tests..