Video capture should set visible size and coded size separately.

Video capture should set visible size and coded size separately.

The coded size should always be rounded up to multiples of 16 pixels to accomodate
alignment requirements of HW encoders, while the visible size reflects the actual
content size.
    
BUG=402151
TEST=verify that non-multiple-of-16-pixel capture works and no black border is shown

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288565

[hshi1, 2014-08-08 22:20:44]

PTAL. As branching point is near, I'd suggest the above short-term fix for M38
to simply round up the desktop capture buffer size to multiples of 16 pixels,
instead of doing an extra copy.

Note that this only rounds up the buffer size; the visible rect is still the
original desktop size, so no upscaling takes place. We will get an 8-pixel black
vertical strip at the right edge of the buffer. I think it is still acceptable
for M38 as it is not very noticeable and does not degrade quality.

[hshi1, 2014-08-08 22:21:14]

Forgot to add Pawel

[Pawel Osciak, 2014-08-08 23:15:10]

lgtm as a standalone patch, I don't understand the interactions well enough
though.

[Alpha Left Google, 2014-08-09 00:06:59]

On 2014/08/08 23:15:10, Pawel Osciak wrote:
> lgtm as a standalone patch, I don't understand the interactions well enough
> though.

The code in browser side can separate coded size and visible size but not the
code in the renderer:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

It needs a bit more plumbing to pass that information on.

[hshi1, 2014-08-09 00:20:29]

On 2014/08/09 00:06:59, Alpha wrote:
> On 2014/08/08 23:15:10, Pawel Osciak wrote:
> > lgtm as a standalone patch, I don't understand the interactions well enough
> > though.
> 
> The code in browser side can separate coded size and visible size but not the
> code in the renderer:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
> 
> It needs a bit more plumbing to pass that information on.

Yes we probably need to pass separate coded/visible size info in
VideoCaptureFormat, but it is too risky a change for M38.

The current change works even though renderer side does not separate coded size
and visible size, because the encoder is initialized correctly with the visible
size.

[Alpha Left Google, 2014-08-09 01:12:27]

On 2014/08/09 00:20:29, hshi1 wrote:
> On 2014/08/09 00:06:59, Alpha wrote:
> > On 2014/08/08 23:15:10, Pawel Osciak wrote:
> > > lgtm as a standalone patch, I don't understand the interactions well
enough
> > > though.
> > 
> > The code in browser side can separate coded size and visible size but not
the
> > code in the renderer:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
> > 
> > It needs a bit more plumbing to pass that information on.
> 
> Yes we probably need to pass separate coded/visible size info in
> VideoCaptureFormat, but it is too risky a change for M38.
> 
> The current change works even though renderer side does not separate coded
size
> and visible size, because the encoder is initialized correctly with the
visible
> size.

LGTM based on our offline discussion. hshi@ will do the plumbing required to
have a separate coded size. This requires changes to IPC and consumers of frames
(i.e. encoders).

[hshi1, 2014-08-09 01:18:54]

The CQ bit was checked by hshi@chromium.org

[commit-bot: I haz the power, 2014-08-09 01:22:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hshi@chromium.org/457823003/20001

[commit-bot: I haz the power, 2014-08-09 10:50:43]

Change committed as 288565

[Alpha Left Google, 2014-08-12 20:30:04]

A revert of this CL has been created in
https://codereview.chromium.org/460283003/ by hclam@chromium.org.

The reason for reverting is: Broke 1080p and 480p tab capture on desktop.
.