Add EncodedImageCallback::OnEncodedImage().

Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/525df3ffd1626efa0dc6be5824bc64297a0e9931
Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Original-Commit-Position: refs/heads/master@{#13615}
Cr-Commit-Position: refs/heads/master@{#13617}

[Sergey Ulanov, 2016-06-21 20:51:33]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2016-06-21 20:53:57]

sergeyu@chromium.org changed reviewers:
+ mflodman@webrtc.org, pbos@webrtc.org

[Sergey Ulanov, 2016-06-24 00:52:43]

pbos, mflodman: ping

[pbos-webrtc, 2016-06-27 22:25:17]

pbos@webrtc.org changed reviewers:
+ sprang@webrtc.org, stefan@webrtc.org

[pbos-webrtc, 2016-06-27 22:25:18]

sprang@/stefan@, are you more involved into these measurements? Changing the
interface for all VideoEncoders is involved so I want to make sure you agree for
this one.

[sprang_webrtc, 2016-06-28 13:54:18]

Some minor comments.
I think this is an improvement overall, but I'm hoping the drop_next_frame flag
will be deprecated in the not too distant future.

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/video_coding_impl.cc (right):

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/video_coding_impl.cc:70: EncodedImageCallback*
callback_ GUARDED_BY(cs_) = nullptr;
Why remove this initialization from the constructor?

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h
File webrtc/video_encoder.h (right):

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:18: #include "webrtc/base/timedelta.h"
Is this import used?

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:41: bool dropped = false;
I "dropped" the right name for this? From what I can see this is more of an
error state than simply a silent frame drop. Maybe even have an enum for
potential outcomes here?

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:44: };
Please comment how frame_id and drop_next_frame should be interpreted.

Is frame_id always the timestamp or is the assumption just that it is an
arbitrary unique id for that frame?

Never been a fan of reporting output from the frame dropper this way, but I
think we'll have to live with it for a little while more. I'm planning to add
new ways of signaling overuse to the capturing pipeline though, just FYI.

[Sergey Ulanov, 2016-06-29 22:18:53]

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/video_coding_impl.cc (right):

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/video_coding_impl.cc:70: EncodedImageCallback*
callback_ GUARDED_BY(cs_) = nullptr;
On 2016/06/28 13:54:18, språng wrote:
> Why remove this initialization from the constructor?

in-class member initialization ensures that the member is always initialized by
all constructors. It's harder to do with initializer lists in constructor
particularly when there are multiple constructors. It also makes code more
readable and easier to maintain as one doesn't have to remember to update
constructor when adding a class member.

I can move it back if you disagree.

BTW, I have a cleanup CL pending here, that adds in-class initializers in other
places: https://codereview.chromium.org/2067673004/

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h
File webrtc/video_encoder.h (right):

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:18: #include "webrtc/base/timedelta.h"
On 2016/06/28 13:54:18, språng wrote:
> Is this import used?

Done.

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:41: bool dropped = false;
On 2016/06/28 13:54:18, språng wrote:
> I "dropped" the right name for this? From what I can see this is more of an
> error state than simply a silent frame drop. Maybe even have an enum for
> potential outcomes here?

Yes, good point. Done. For now I just added two possible result codes: OK, and
ERROR_SEND_FAILED.

https://codereview.webrtc.org/2089773002/diff/20001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:44: };
On 2016/06/28 13:54:18, språng wrote:
> Please comment how frame_id and drop_next_frame should be interpreted.

Done.

> 
> Is frame_id always the timestamp or is the assumption just that it is an
> arbitrary unique id for that frame?

It depends on the underlying protocol used to send video, doesn't have to be a
timestamp. The only thing that matters is that both With RTP we can use frame
timestamp as frame_id (mentioned it in the comment now).


> 
> Never been a fan of reporting output from the frame dropper this way, but I
> think we'll have to live with it for a little while more. I'm planning to add
> new ways of signaling overuse to the capturing pipeline though, just FYI.

ok. please keep me up to date, maybe cc on the relevant CLs.

[sprang_webrtc, 2016-07-01 15:39:32]

lgtm

[Sergey Ulanov, 2016-07-13 04:58:03]

stefan@webrtc.org: ping

[stefan-webrtc, 2016-07-18 16:51:27]

Overall I think this is good

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h:92: return
Result(Result::OK, picture_id_);
Seems a bit surprising that we return picture_id_ here and not the timestamp?

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/video_coding_impl.cc (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/video_coding_impl.cc:70: EncodedImageCallback*
callback_ GUARDED_BY(cs_) = nullptr;
Prefer if you set this in the constructor initializer list.

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
File webrtc/video/payload_router.cc (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
webrtc/video/payload_router.cc:174: rtp_modules_[stream_index]->StartTimestamp()
+ encoded_image._timeStamp;
Would it make sense to instead have SendOutgoingData return the timestamp that
will be used?

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video_encoder.h
File webrtc/video_encoder.h (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:49: uint32_t frame_id = 0;
Default value should probably be -1, or if you prefer to make this an Optional,
since it's not required to pass an ID. Or should the user assume that it's not
set if error != OK? Add that to the comment in that case.

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:51: // Tells the encoder that the next frame is going to
be dropped.
Maybe "should be dropped" instead?

[Sergey Ulanov, 2016-07-21 00:09:40]

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h:92: return
Result(Result::OK, picture_id_);
On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> Seems a bit surprising that we return picture_id_ here and not the timestamp?

Changed it to return timetstamp_, but I don't think it actually matters for this
test.

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/video_coding_impl.cc (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/video_coding_impl.cc:70: EncodedImageCallback*
callback_ GUARDED_BY(cs_) = nullptr;
On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> Prefer if you set this in the constructor initializer list.

Done.

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
File webrtc/video/payload_router.cc (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
webrtc/video/payload_router.cc:174: rtp_modules_[stream_index]->StartTimestamp()
+ encoded_image._timeStamp;
On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> Would it make sense to instead have SendOutgoingData return the timestamp that
> will be used?

Yes, but RtpRtcp::SendOutgoingData() is used in bunch of places outside of
webrtc, so it's hard to change it, so this seems to be the least bad solution.
Or I could add a second version SendOutgoingData() that would also return
timestamp, WDYT?

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video_encoder.h
File webrtc/video_encoder.h (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:49: uint32_t frame_id = 0;
On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> Default value should probably be -1, or if you prefer to make this an
Optional,
> since it's not required to pass an ID. Or should the user assume that it's not
> set if error != OK? Add that to the comment in that case.

it cannot be -1 because it's unsigned, and timestamp in RTP may be any 32-bit
value, so we cannot reserve a special value for null ID. The idea here is that
it's aloways initialized as long as error = OK.

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video_encoder.h#ne...
webrtc/video_encoder.h:51: // Tells the encoder that the next frame is going to
be dropped.
On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> Maybe "should be dropped" instead?

Done.

[Sergey Ulanov, 2016-07-22 18:05:19]

stefan@webrtc.org: ping

[stefan-webrtc, 2016-07-25 15:17:09]

lgtm with nit fixed.

https://codereview.chromium.org/2089773002/diff/60001/webrtc/video_encoder.h
File webrtc/video_encoder.h (right):

https://codereview.chromium.org/2089773002/diff/60001/webrtc/video_encoder.h#...
webrtc/video_encoder.h:49: uint32_t frame_id = 0;
On 2016/07/21 00:09:40, Sergey Ulanov wrote:
> On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> > Default value should probably be -1, or if you prefer to make this an
> Optional,
> > since it's not required to pass an ID. Or should the user assume that it's
not
> > set if error != OK? Add that to the comment in that case.
> 
> it cannot be -1 because it's unsigned, and timestamp in RTP may be any 32-bit
> value, so we cannot reserve a special value for null ID. The idea here is that
> it's aloways initialized as long as error = OK.

You could still use an Optional<uint32_t> or an int64_t with -1 representing
uninitialized. But I think the current choice probably is good enough.

https://codereview.chromium.org/2089773002/diff/80001/webrtc/modules/video_co...
File webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h (right):

https://codereview.chromium.org/2089773002/diff/80001/webrtc/modules/video_co...
webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h:92: return
Result(Result::OK, encoded_image._timeS  tamp.);
This looks weird...

[stefan-webrtc, 2016-07-25 15:19:33]

not lgtm

forgot to reply to one comment...

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
File webrtc/video/payload_router.cc (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
webrtc/video/payload_router.cc:174: rtp_modules_[stream_index]->StartTimestamp()
+ encoded_image._timeStamp;
On 2016/07/21 00:09:39, Sergey Ulanov wrote:
> On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> > Would it make sense to instead have SendOutgoingData return the timestamp
that
> > will be used?
> 
> Yes, but RtpRtcp::SendOutgoingData() is used in bunch of places outside of
> webrtc, so it's hard to change it, so this seems to be the least bad solution.
> Or I could add a second version SendOutgoingData() that would also return
> timestamp, WDYT?

Missed this comment, sorry.

I would prefer to add a secibd versuib if SendOutgoingData() and deprecate the
old.

[Sergey Ulanov, 2016-07-26 19:19:53]

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
File webrtc/video/payload_router.cc (right):

https://codereview.webrtc.org/2089773002/diff/60001/webrtc/video/payload_rout...
webrtc/video/payload_router.cc:174: rtp_modules_[stream_index]->StartTimestamp()
+ encoded_image._timeStamp;
On 2016/07/25 15:19:33, stefan-webrtc (holmer) wrote:
> On 2016/07/21 00:09:39, Sergey Ulanov wrote:
> > On 2016/07/18 16:51:27, stefan-webrtc (holmer) wrote:
> > > Would it make sense to instead have SendOutgoingData return the timestamp
> that
> > > will be used?
> > 
> > Yes, but RtpRtcp::SendOutgoingData() is used in bunch of places outside of
> > webrtc, so it's hard to change it, so this seems to be the least bad
solution.
> > Or I could add a second version SendOutgoingData() that would also return
> > timestamp, WDYT?
> 
> Missed this comment, sorry.
> 
> I would prefer to add a secibd versuib if SendOutgoingData() and deprecate the
> old.

Done

[stefan-webrtc, 2016-07-28 08:51:17]

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:318: // Deprecated version of the
methood above.
method

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:451: int32_t
RTPSender::SendOutgoingData(FrameType frame_type,
As you are anyway introducing this new method, maybe you can change it to return
bool instead of int32_t?

Or maybe even return the new Result type from here too?

[Sergey Ulanov, 2016-07-28 20:52:20]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 20:52:22]

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

[Sergey Ulanov, 2016-07-28 20:52:27]

The CQ bit was unchecked by sergeyu@chromium.org

[Sergey Ulanov, 2016-07-28 20:52:46]

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:318: // Deprecated version of the
methood above.
On 2016/07/28 08:51:17, stefan-webrtc (holmer) wrote:
> method

Done.

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:451: int32_t
RTPSender::SendOutgoingData(FrameType frame_type,
On 2016/07/28 08:51:17, stefan-webrtc (holmer) wrote:
> As you are anyway introducing this new method, maybe you can change it to
return
> bool instead of int32_t?

Done.

> 
> Or maybe even return the new Result type from here too?

That would require moving Result struct somewhere where it can be shared with
this class.

[stefan-webrtc, 2016-07-29 07:13:37]

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:451: int32_t
RTPSender::SendOutgoingData(FrameType frame_type,
On 2016/07/28 20:52:46, Sergey Ulanov wrote:
> On 2016/07/28 08:51:17, stefan-webrtc (holmer) wrote:
> > As you are anyway introducing this new method, maybe you can change it to
> return
> > bool instead of int32_t?
> 
> Done.
> 
> > 
> > Or maybe even return the new Result type from here too?
> 
> That would require moving Result struct somewhere where it can be shared with
> this class.

Sure, maybe module_common_types.h?

[Sergey Ulanov, 2016-08-01 07:22:13]

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:451: int32_t
RTPSender::SendOutgoingData(FrameType frame_type,
On 2016/07/29 07:13:37, stefan-webrtc (holmer) wrote:
> On 2016/07/28 20:52:46, Sergey Ulanov wrote:
> > On 2016/07/28 08:51:17, stefan-webrtc (holmer) wrote:
> > > As you are anyway introducing this new method, maybe you can change it to
> > return
> > > bool instead of int32_t?
> > 
> > Done.
> > 
> > > 
> > > Or maybe even return the new Result type from here too?
> > 
> > That would require moving Result struct somewhere where it can be shared
with
> > this class.
> 
> Sure, maybe module_common_types.h?

Another issue is that we need to keep the current version of
RtpRtcp::SendOutgoingData() and it's not possible to overload it with new return
type, without changing args. Are there any other parameters that can be updated?

[stefan-webrtc, 2016-08-01 07:29:05]

On 2016/08/01 07:22:13, Sergey Ulanov wrote:
>
https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
> File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):
> 
>
https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
> webrtc/modules/rtp_rtcp/source/rtp_sender.cc:451: int32_t
> RTPSender::SendOutgoingData(FrameType frame_type,
> On 2016/07/29 07:13:37, stefan-webrtc (holmer) wrote:
> > On 2016/07/28 20:52:46, Sergey Ulanov wrote:
> > > On 2016/07/28 08:51:17, stefan-webrtc (holmer) wrote:
> > > > As you are anyway introducing this new method, maybe you can change it
to
> > > return
> > > > bool instead of int32_t?
> > > 
> > > Done.
> > > 
> > > > 
> > > > Or maybe even return the new Result type from here too?
> > > 
> > > That would require moving Result struct somewhere where it can be shared
> with
> > > this class.
> > 
> > Sure, maybe module_common_types.h?
> 
> Another issue is that we need to keep the current version of
> RtpRtcp::SendOutgoingData() and it's not possible to overload it with new
return
> type, without changing args. Are there any other parameters that can be
updated?

Ok, that's a bit annoying. Maybe make the payload_type an int. Or you may have
to go with you already had... If the latter, maybe change the variable name to
transport_frame_id instead of rtp_timestamp_out to match what the other CL did?

[Sergey Ulanov, 2016-08-02 00:18:00]

On 2016/08/01 07:29:05, stefan-webrtc (holmer) wrote:
> On 2016/08/01 07:22:13, Sergey Ulanov wrote:
> >
>
https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
> > File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):
> > 
> >
>
https://codereview.webrtc.org/2089773002/diff/120001/webrtc/modules/rtp_rtcp/...
> > webrtc/modules/rtp_rtcp/source/rtp_sender.cc:451: int32_t
> > RTPSender::SendOutgoingData(FrameType frame_type,
> > On 2016/07/29 07:13:37, stefan-webrtc (holmer) wrote:
> > > On 2016/07/28 20:52:46, Sergey Ulanov wrote:
> > > > On 2016/07/28 08:51:17, stefan-webrtc (holmer) wrote:
> > > > > As you are anyway introducing this new method, maybe you can change it
> to
> > > > return
> > > > > bool instead of int32_t?
> > > > 
> > > > Done.
> > > > 
> > > > > 
> > > > > Or maybe even return the new Result type from here too?
> > > > 
> > > > That would require moving Result struct somewhere where it can be shared
> > with
> > > > this class.
> > > 
> > > Sure, maybe module_common_types.h?
> > 
> > Another issue is that we need to keep the current version of
> > RtpRtcp::SendOutgoingData() and it's not possible to overload it with new
> return
> > type, without changing args. Are there any other parameters that can be
> updated?
> 
> Ok, that's a bit annoying. Maybe make the payload_type an int.

I think that would be acceptable as it would make it very hard to figure out
which of the two overloads is getting called.

> Or you may have
> to go with you already had... If the latter, maybe change the variable name to
> transport_frame_id instead of rtp_timestamp_out to match what the other CL
did?

Done.

[stefan-webrtc, 2016-08-02 06:33:37]

lgtm

[Sergey Ulanov, 2016-08-02 17:14:40]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 17:14:51]

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, 2016-08-02 17:16:57]

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

[commit-bot: I haz the power, 2016-08-02 17:16:58]

Dry run: Try jobs failed on following builders:
  ios64_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/9258)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/11574)
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/11503)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/15459)
  linux_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/16915)
  linux_ubsan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/3787)

[Sergey Ulanov, 2016-08-02 18:38:37]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-08-02 18:38:38]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org,
stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2089773002/#ps220001 (title:
".")

[commit-bot: I haz the power, 2016-08-02 18:38:44]

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, 2016-08-02 18:41:53]

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

[commit-bot: I haz the power, 2016-08-02 18:41:54]

Try jobs failed on following builders:
  win_drmemory_light on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_drmemory_light/buil...)
  win_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/16784)

[Sergey Ulanov, 2016-08-02 18:44:22]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 18:44:33]

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, 2016-08-02 18:49:23]

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

[commit-bot: I haz the power, 2016-08-02 18:49:24]

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

[Sergey Ulanov, 2016-08-02 19:26:49]

Patchset #12 (id:240001) has been deleted

[Sergey Ulanov, 2016-08-02 19:26:57]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 19:27:09]

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

[Sergey Ulanov, 2016-08-02 19:29:09]

sergeyu@chromium.org changed reviewers:
+ niklas.enbom@webrtc.org

[Sergey Ulanov, 2016-08-02 19:29:10]

+niklas.enbom@webrtc.org
Niklas, can you please approve this change?

[commit-bot: I haz the power, 2016-08-02 19:29:59]

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

[commit-bot: I haz the power, 2016-08-02 19:30:00]

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

[Sergey Ulanov, 2016-08-02 19:57:18]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 19:57:22]

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, 2016-08-02 20:11:14]

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

[commit-bot: I haz the power, 2016-08-02 20:11:15]

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

[niklas.enbom, 2016-08-02 20:16:59]

RS lgtm
On 2016/08/02 19:29:10, Sergey Ulanov wrote:
> mailto:+niklas.enbom@webrtc.org
> Niklas, can you please approve this change?

[Sergey Ulanov, 2016-08-02 20:44:30]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
==========

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

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

[commit-bot: I haz the power, 2016-08-02 20:44:31]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

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

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

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

[Sergey Ulanov, 2016-08-02 20:44:33]

Committed patchset #13 (id:280001) manually as
ad34dbe934d47f88011045671b4aea00dbd5a795 (presubmit successful).

[commit-bot: I haz the power, 2016-08-02 20:44:33]

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

[Sergey Ulanov, 2016-08-02 21:32:43]

A revert of this CL (patchset #13 id:280001) has been created in
https://codereview.webrtc.org/2206743002/ by sergeyu@chromium.org.

The reason for reverting is: broke internal tests.

[Sergey Ulanov, 2016-08-02 21:33:30]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

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

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

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

[Sergey Ulanov, 2016-08-02 21:59:19]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-08-02 21:59:20]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org,
stefan@webrtc.org, niklas.enbom@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2089773002/#ps300001 (title:
".")

[commit-bot: I haz the power, 2016-08-02 21:59:28]

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, 2016-08-02 22:06:23]

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

[commit-bot: I haz the power, 2016-08-02 22:06:25]

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

[Sergey Ulanov, 2016-08-02 22:14:58]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

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

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Commit-Position: refs/heads/master@{#13613}
Committed:
https://chromium.googlesource.com/external/webrtc/+/4c7f4cd2ef76821edca6d773d...
==========

[Sergey Ulanov, 2016-08-02 22:15:00]

Committed patchset #14 (id:300001) manually as
4c7f4cd2ef76821edca6d773d733a924b0bedd25 (presubmit successful).

[commit-bot: I haz the power, 2016-08-02 22:15:03]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Commit-Position: refs/heads/master@{#13613}
Committed:
https://chromium.googlesource.com/external/webrtc/+/4c7f4cd2ef76821edca6d773d...
==========

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Commit-Position: refs/heads/master@{#13615}
==========

[Sergey Ulanov, 2016-08-03 00:33:13]

A revert of this CL (patchset #14 id:300001) has been created in
https://codereview.chromium.org/2203233002/ by sergeyu@chromium.org.

The reason for reverting is: broke browser_tests.

[Sergey Ulanov, 2016-08-03 00:35:36]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Commit-Position: refs/heads/master@{#13615}
==========

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Commit-Position: refs/heads/master@{#13615}
==========

[Sergey Ulanov, 2016-08-03 00:37:25]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-08-03 00:37:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
niklas.enbom@webrtc.org, stefan@webrtc.org, sprang@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2089773002/#ps320001 (title:
".")

[commit-bot: I haz the power, 2016-08-03 00:37:36]

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

[Sergey Ulanov, 2016-08-03 00:46:52]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Commit-Position: refs/heads/master@{#13615}
==========

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Commit-Position: refs/heads/master@{#13615}
Committed:
https://chromium.googlesource.com/external/webrtc/+/525df3ffd1626efa0dc6be582...
==========

[commit-bot: I haz the power, 2016-08-03 00:46:53]

Description was changed from

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Commit-Position: refs/heads/master@{#13615}
==========

to

==========
Add EncodedImageCallback::OnEncodedImage().

OnEncodedImage() is going to replace Encoded(), which is deprecated now.
The new OnEncodedImage() returns Result struct that contains frame_id,
which tells the encoder RTP timestamp for the frame.

BUG=chromium:621691
R=niklas.enbom@webrtc.org, sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/525df3ffd1626efa0dc6be5824bc64297a0e9931
Committed: https://crrev.com/4c7f4cd2ef76821edca6d773d733a924b0bedd25
Committed: https://crrev.com/ad34dbe934d47f88011045671b4aea00dbd5a795
Cr-Original-Original-Commit-Position: refs/heads/master@{#13613}
Cr-Original-Commit-Position: refs/heads/master@{#13615}
Cr-Commit-Position: refs/heads/master@{#13617}
==========

[Sergey Ulanov, 2016-08-03 00:46:54]

Committed patchset #15 (id:320001) manually as
525df3ffd1626efa0dc6be5824bc64297a0e9931 (presubmit successful).

[commit-bot: I haz the power, 2016-08-03 00:46:55]

Patchset 15 (id:??) landed as
https://crrev.com/525df3ffd1626efa0dc6be5824bc64297a0e9931
Cr-Commit-Position: refs/heads/master@{#13617}