Return WrappedI444Buffer in VP9Impl

Return WrappedI444Buffer in VP9Impl

This CL allows VP9Impl to return a WrappedI444Buffer if the image format is
VPX_IMG_FMT_I444.

Needs to be checked in after Chromium CL 2876363003 is checked in.

BUG=webrtc:7506
Review-Url: https://codereview.webrtc.org/2927943003
Cr-Commit-Position: refs/heads/master@{#18727}
Committed: https://chromium.googlesource.com/external/webrtc/+/23cc468ddf733ab89721338a4d916866f5235716

[Yuwei, 2017-06-08 20:38:37]

yuweih@chromium.org changed reviewers:
+ magjed@webrtc.org, nisse@webrtc.org

[Yuwei, 2017-06-08 20:38:37]

PTAL thanks!

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:84: class WrappedI444Buffer :
public I444BufferInterface {
TBH I don't quite like duplicating the WrappedI420Buffer class but I couldn't
think of better way to work with the I444BufferInterface...

[nisse-webrtc, 2017-06-09 09:01:22]

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:84: class WrappedI444Buffer :
public I444BufferInterface {
On 2017/06/08 20:38:37, Yuwei wrote:
> TBH I don't quite like duplicating the WrappedI420Buffer class but I couldn't
> think of better way to work with the I444BufferInterface...

I think it should be doable to define WrappedYuvBuffer as a template,
parameterized by the desired interface class (and hence indirectly by the type
enum). So we could define

  using WrappedI420Buffer = WrappedYuvBuffer<I420BufferInterface>;

  using WrappedI444Buffer = WrappedYuvBuffer<I444BufferInterface>;

Not sure if it's worth the extra complexity, though?

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:972: img_wrapped_buffer = new
rtc::RefCountedObject<webrtc::WrappedI444Buffer>(
It's a pity classes aren't first class in C++, but maybe we can use static
Create methods as a substitute? They're nice to have anyway, to omit the
RefCountedObject boilerplate. So maybe this could be written more clearly and
concisely as 

img_wrapped_buffer = 
    (type == VPX_IMG_FMR_I444 
        ? WrappedI444Buffer::Create 
        : WrappedI420Buffer::Create) (
    ... the identical construction args...);

[magjed_webrtc, 2017-06-09 13:46:57]

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:84: class WrappedI444Buffer :
public I444BufferInterface {
On 2017/06/09 09:01:22, nisse-webrtc wrote:
> On 2017/06/08 20:38:37, Yuwei wrote:
> > TBH I don't quite like duplicating the WrappedI420Buffer class but I
couldn't
> > think of better way to work with the I444BufferInterface...
> 
> I think it should be doable to define WrappedYuvBuffer as a template,
> parameterized by the desired interface class (and hence indirectly by the type
> enum). So we could define
> 
>   using WrappedI420Buffer = WrappedYuvBuffer<I420BufferInterface>;
> 
>   using WrappedI444Buffer = WrappedYuvBuffer<I444BufferInterface>;
> 
> Not sure if it's worth the extra complexity, though?

How about we expose two factory functions instead:
rtc::scoped_refptr<I420BufferInterface> WrapI420(int width, int height, ...);
rtc::scoped_refptr<I444BufferInterface> WrapI444(int width, int height, ...);
and do the template implementation in the .cc file? I don't want to expose the
template implementation here, but I'm fine with doing it in the .cc file.

[Yuwei, 2017-06-10 00:04:04]

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

[commit-bot: I haz the power, 2017-06-10 00:04:18]

Dry run: CQ is trying da patch.

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

[Yuwei, 2017-06-10 00:13:53]

PTAL thanks!

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:84: class WrappedI444Buffer :
public I444BufferInterface {
On 2017/06/09 13:46:57, magjed_webrtc wrote:
> On 2017/06/09 09:01:22, nisse-webrtc wrote:
> > On 2017/06/08 20:38:37, Yuwei wrote:
> > > TBH I don't quite like duplicating the WrappedI420Buffer class but I
> couldn't
> > > think of better way to work with the I444BufferInterface...
> > 
> > I think it should be doable to define WrappedYuvBuffer as a template,
> > parameterized by the desired interface class (and hence indirectly by the
type
> > enum). So we could define
> > 
> >   using WrappedI420Buffer = WrappedYuvBuffer<I420BufferInterface>;
> > 
> >   using WrappedI444Buffer = WrappedYuvBuffer<I444BufferInterface>;
> > 
> > Not sure if it's worth the extra complexity, though?
> 
> How about we expose two factory functions instead:
> rtc::scoped_refptr<I420BufferInterface> WrapI420(int width, int height, ...);
> rtc::scoped_refptr<I444BufferInterface> WrapI444(int width, int height, ...);
> and do the template implementation in the .cc file? I don't want to expose the
> template implementation here, but I'm fine with doing it in the .cc file.

Done. This is much better :) But please also see the comment below.

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:972: img_wrapped_buffer = new
rtc::RefCountedObject<webrtc::WrappedI444Buffer>(
On 2017/06/09 09:01:22, nisse-webrtc wrote:
> It's a pity classes aren't first class in C++, but maybe we can use static
> Create methods as a substitute? They're nice to have anyway, to omit the
> RefCountedObject boilerplate. So maybe this could be written more clearly and
> concisely as 
> 
> img_wrapped_buffer = 
>     (type == VPX_IMG_FMR_I444 
>         ? WrappedI444Buffer::Create 
>         : WrappedI420Buffer::Create) (
>     ... the identical construction args...);
> 

Unfortunately function that returns scoped_refptr<I420BufferInterface> is not
thought to be compatible with function that returns
scoped_refptr<I444BufferInterface> when applying ternary operator...


If we don't care about the I4XXBufferInterface information then we could simply
implement the factory function like this:

scoped_refptr<VideoFrameBuffer> WrapYuvBuffer(width, height, ..., pixel_format);

rather than having WrapI420 and WrapI444 separately.

Given that other uses of WrappedI420Buffer only treat it as a VideoFrameBuffer
and VideoFrameBuffer has a Type enum, it should be fine to do so?

[commit-bot: I haz the power, 2017-06-10 00:27:17]

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

[commit-bot: I haz the power, 2017-06-10 00:27:17]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2017-06-13 11:38:27]

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:972: img_wrapped_buffer = new
rtc::RefCountedObject<webrtc::WrappedI444Buffer>(
On 2017/06/10 00:13:53, Yuwei wrote:
> On 2017/06/09 09:01:22, nisse-webrtc wrote:
> > It's a pity classes aren't first class in C++, but maybe we can use static
> > Create methods as a substitute? They're nice to have anyway, to omit the
> > RefCountedObject boilerplate. So maybe this could be written more clearly
and
> > concisely as 
> > 
> > img_wrapped_buffer = 
> >     (type == VPX_IMG_FMR_I444 
> >         ? WrappedI444Buffer::Create 
> >         : WrappedI420Buffer::Create) (
> >     ... the identical construction args...);
> > 
> 
> Unfortunately function that returns scoped_refptr<I420BufferInterface> is not
> thought to be compatible with function that returns
> scoped_refptr<I444BufferInterface> when applying ternary operator...
> 
> 
> If we don't care about the I4XXBufferInterface information then we could
simply
> implement the factory function like this:
> 
> scoped_refptr<VideoFrameBuffer> WrapYuvBuffer(width, height, ...,
pixel_format);
> 
> rather than having WrapI420 and WrapI444 separately.
> 
> Given that other uses of WrappedI420Buffer only treat it as a VideoFrameBuffer
> and VideoFrameBuffer has a Type enum, it should be fine to do so?

Yeah, I would like to spare users this duplication. Can you add an extra
function alongside WrapI420 and WrapI444 like this:
rtc::scoped_refptr<PlanarYuvBuffer> WrapYuvBuffer(Type type, width, height, ...)
that will call either WrapI420 or WrapI444 depending on |type|?

[nisse-webrtc, 2017-06-13 12:29:09]

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:972: img_wrapped_buffer = new
rtc::RefCountedObject<webrtc::WrappedI444Buffer>(
On 2017/06/10 00:13:53, Yuwei wrote:
> On 2017/06/09 09:01:22, nisse-webrtc wrote:
> > It's a pity classes aren't first class in C++, but maybe we can use static
> > Create methods as a substitute? They're nice to have anyway, to omit the
> > RefCountedObject boilerplate. So maybe this could be written more clearly
and
> > concisely as 
> > 
> > img_wrapped_buffer = 
> >     (type == VPX_IMG_FMR_I444 
> >         ? WrappedI444Buffer::Create 
> >         : WrappedI420Buffer::Create) (
> >     ... the identical construction args...);
> > 
> 
> Unfortunately function that returns scoped_refptr<I420BufferInterface> is not
> thought to be compatible with function that returns
> scoped_refptr<I444BufferInterface> when applying ternary operator...

Ouch. One would need to wrap them in a lambda or std::function with the intended
return type, then. Which is no longer so nice and concise.

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:972: img_wrapped_buffer = new
rtc::RefCountedObject<webrtc::WrappedI444Buffer>(
On 2017/06/13 11:38:27, magjed_webrtc wrote:

> Yeah, I would like to spare users this duplication. Can you add an extra
> function alongside WrapI420 and WrapI444 like this:
> rtc::scoped_refptr<PlanarYuvBuffer> WrapYuvBuffer(Type type, width, height,
...)
> that will call either WrapI420 or WrapI444 depending on |type|?

Sounds good to me too.

https://codereview.webrtc.org/2927943003/diff/20001/webrtc/common_video/inclu...
File webrtc/common_video/include/video_frame_buffer.h (left):

https://codereview.webrtc.org/2927943003/diff/20001/webrtc/common_video/inclu...
webrtc/common_video/include/video_frame_buffer.h:48: class WrappedI420Buffer :
public I420BufferInterface {
Deleting this class will likely break some downstream apps (including Chrome?).
So I suspect you'll need a multiple steps to land this, where the first step
would be adding the new WrapI420Buffer function.

[Yuwei, 2017-06-19 21:22:15]

Patchset #4 (id:60001) has been deleted

[Yuwei, 2017-06-19 21:22:21]

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

[commit-bot: I haz the power, 2017-06-19 21:22:30]

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-06-19 21:23:59]

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

[commit-bot: I haz the power, 2017-06-19 21:24:00]

Dry run: Try jobs failed on following builders:
  ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...)
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/21305)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/26625)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/25597)

[Yuwei, 2017-06-19 21:33:26]

Sorry for the delay.. PTAL thanks!

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:972: img_wrapped_buffer = new
rtc::RefCountedObject<webrtc::WrappedI444Buffer>(
On 2017/06/13 11:38:27, magjed_webrtc wrote:
> On 2017/06/10 00:13:53, Yuwei wrote:
> > On 2017/06/09 09:01:22, nisse-webrtc wrote:
> > > It's a pity classes aren't first class in C++, but maybe we can use static
> > > Create methods as a substitute? They're nice to have anyway, to omit the
> > > RefCountedObject boilerplate. So maybe this could be written more clearly
> and
> > > concisely as 
> > > 
> > > img_wrapped_buffer = 
> > >     (type == VPX_IMG_FMR_I444 
> > >         ? WrappedI444Buffer::Create 
> > >         : WrappedI420Buffer::Create) (
> > >     ... the identical construction args...);
> > > 
> > 
> > Unfortunately function that returns scoped_refptr<I420BufferInterface> is
not
> > thought to be compatible with function that returns
> > scoped_refptr<I444BufferInterface> when applying ternary operator...
> > 
> > 
> > If we don't care about the I4XXBufferInterface information then we could
> simply
> > implement the factory function like this:
> > 
> > scoped_refptr<VideoFrameBuffer> WrapYuvBuffer(width, height, ...,
> pixel_format);
> > 
> > rather than having WrapI420 and WrapI444 separately.
> > 
> > Given that other uses of WrappedI420Buffer only treat it as a
VideoFrameBuffer
> > and VideoFrameBuffer has a Type enum, it should be fine to do so?
> 
> Yeah, I would like to spare users this duplication. Can you add an extra
> function alongside WrapI420 and WrapI444 like this:
> rtc::scoped_refptr<PlanarYuvBuffer> WrapYuvBuffer(Type type, width, height,
...)
> that will call either WrapI420 or WrapI444 depending on |type|?

Done.

https://codereview.webrtc.org/2927943003/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:972: img_wrapped_buffer = new
rtc::RefCountedObject<webrtc::WrappedI444Buffer>(
On 2017/06/13 12:29:09, nisse-webrtc wrote:
> On 2017/06/10 00:13:53, Yuwei wrote:
> > On 2017/06/09 09:01:22, nisse-webrtc wrote:
> > > It's a pity classes aren't first class in C++, but maybe we can use static
> > > Create methods as a substitute? They're nice to have anyway, to omit the
> > > RefCountedObject boilerplate. So maybe this could be written more clearly
> and
> > > concisely as 
> > > 
> > > img_wrapped_buffer = 
> > >     (type == VPX_IMG_FMR_I444 
> > >         ? WrappedI444Buffer::Create 
> > >         : WrappedI420Buffer::Create) (
> > >     ... the identical construction args...);
> > > 
> > 
> > Unfortunately function that returns scoped_refptr<I420BufferInterface> is
not
> > thought to be compatible with function that returns
> > scoped_refptr<I444BufferInterface> when applying ternary operator...
> 
> Ouch. One would need to wrap them in a lambda or std::function with the
intended
> return type, then. Which is no longer so nice and concise. 

Added the WrapYuvBuffer function.

Functional tricks may just be too complex in this case. I think the switch in
WrapYuvBuffer is manageable enough.

https://codereview.webrtc.org/2927943003/diff/20001/webrtc/common_video/inclu...
File webrtc/common_video/include/video_frame_buffer.h (left):

https://codereview.webrtc.org/2927943003/diff/20001/webrtc/common_video/inclu...
webrtc/common_video/include/video_frame_buffer.h:48: class WrappedI420Buffer :
public I420BufferInterface {
On 2017/06/13 12:29:09, nisse-webrtc wrote:
> Deleting this class will likely break some downstream apps (including
Chrome?).
> So I suspect you'll need a multiple steps to land this, where the first step
> would be adding the new WrapI420Buffer function.
>  

Just added back WrappedI420Buffer. I did a cs on Chromium and doesn't seem it's
being used outside WebRTC, but I think it's still good to break it down into
smaller steps.

BTW does it have any downstream other than Chromium?

[Yuwei, 2017-06-19 21:46:32]

Patchset #5 (id:100001) has been deleted

[Yuwei, 2017-06-19 21:47:18]

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

[commit-bot: I haz the power, 2017-06-19 21:47:19]

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-06-19 21:51:29]

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

[commit-bot: I haz the power, 2017-06-19 21:51:30]

Dry run: Try jobs failed on following builders:
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/21098)
  win_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_dbg/builds/19939)
  win_x64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/26550)

[Yuwei, 2017-06-19 21:55:49]

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

[commit-bot: I haz the power, 2017-06-19 21:55:59]

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-06-19 22:21:16]

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

[commit-bot: I haz the power, 2017-06-19 22:21:17]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2017-06-20 09:01:15]

lgtm

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:973: // using a
WrappedI4??Buffer.
Use WrapYuvBuffer instead of WrappedI4??Buffer now

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:979: // WrappedI4??Buffer's
mechanism for allowing the release of its frame
ditto, use WrapYuvBuffer instead of WrappedI4??Buffer now

[nisse-webrtc, 2017-06-20 09:58:47]

lgtm

Please use the linux_internal trybot before landing.

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/common_video/vide...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/common_video/vide...
webrtc/common_video/video_frame_buffer.cc:233: RTC_NOTREACHED();
nit: The pattern of DCHECK (which RTC_NOTREACHED is) + return of an error value
is questionable, for one thing, proper error handling of the error return is
untestable since all our tests run with DCHECKs enabled.

So I'd prefer either FATAL() << "some message" or return nullptr.

[Yuwei, 2017-06-22 01:17:25]

Thanks!

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/common_video/vide...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/common_video/vide...
webrtc/common_video/video_frame_buffer.cc:233: RTC_NOTREACHED();
On 2017/06/20 09:58:47, nisse-webrtc wrote:
> nit: The pattern of DCHECK (which RTC_NOTREACHED is) + return of an error
value
> is questionable, for one thing, proper error handling of the error return is
> untestable since all our tests run with DCHECKs enabled.
> 
> So I'd prefer either FATAL() << "some message" or return nullptr.

Done. I just did both both since the Windows compiler will complain about
missing return value if I don't return nullptr...

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:973: // using a
WrappedI4??Buffer.
On 2017/06/20 09:01:15, magjed_webrtc wrote:
> Use WrapYuvBuffer instead of WrappedI4??Buffer now

Done.

https://codereview.webrtc.org/2927943003/diff/140001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:979: // WrappedI4??Buffer's
mechanism for allowing the release of its frame
On 2017/06/20 09:01:15, magjed_webrtc wrote:
> ditto, use WrapYuvBuffer instead of WrappedI4??Buffer now

Done.

[Yuwei, 2017-06-22 01:17:30]

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

[commit-bot: I haz the power, 2017-06-22 01:17:38]

Dry run: CQ is trying da patch.

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

[Yuwei, 2017-06-22 01:17:48]

The CQ bit was unchecked by yuweih@chromium.org

[Yuwei, 2017-06-22 01:22:55]

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

[commit-bot: I haz the power, 2017-06-22 01:22:59]

Dry run: CQ is trying da patch.

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

[Yuwei, 2017-06-22 01:25:09]

The CQ bit was unchecked by yuweih@chromium.org

[Yuwei, 2017-06-22 07:34:52]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2017-06-22 07:34:52]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
nisse@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2927943003/#ps160001 (title:
"Resolve Feedback")

[commit-bot: I haz the power, 2017-06-22 07:34:56]

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-06-22 07:38:46]

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

[commit-bot: I haz the power, 2017-06-22 07:38:48]

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

[Yuwei, 2017-06-23 02:38:15]

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

[commit-bot: I haz the power, 2017-06-23 02:38:26]

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-06-23 03:07:16]

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

[commit-bot: I haz the power, 2017-06-23 03:07:17]

Dry run: This issue passed the CQ dry run.

[Yuwei, 2017-06-23 03:26:09]

linux_internal passed.

Let's do the WrappedI420Buffer => WrapI420Buffer in a separate CL.

Checking in. Thanks!

[Yuwei, 2017-06-23 03:26:15]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2017-06-23 03:26:15]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
nisse@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2927943003/#ps180001 (title:
"Revert refactoring on other files")

[commit-bot: I haz the power, 2017-06-23 03:26:29]

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-06-23 03:28:07]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1498188375319540,
"parent_rev": "86a49f8b5f9df0039ec999fbb7579dc82f4988f1", "commit_rev":
"23cc468ddf733ab89721338a4d916866f5235716"}

[commit-bot: I haz the power, 2017-06-23 03:28:12]

Description was changed from

==========
Return WrappedI444Buffer in VP9Impl

This CL allows VP9Impl to return a WrappedI444Buffer if the image format is
VPX_IMG_FMT_I444.

Needs to be checked in after Chromium CL 2876363003 is checked in.

BUG=webrtc:7506
==========

to

==========
Return WrappedI444Buffer in VP9Impl

This CL allows VP9Impl to return a WrappedI444Buffer if the image format is
VPX_IMG_FMT_I444.

Needs to be checked in after Chromium CL 2876363003 is checked in.

BUG=webrtc:7506

Review-Url: https://codereview.webrtc.org/2927943003
Cr-Commit-Position: refs/heads/master@{#18727}
Committed:
https://chromium.googlesource.com/external/webrtc/+/23cc468ddf733ab89721338a4...
==========

[commit-bot: I haz the power, 2017-06-23 03:28:13]

Committed patchset #8 (id:180001) as
https://chromium.googlesource.com/external/webrtc/+/23cc468ddf733ab89721338a4...