Add flag to decoder class to flip the image vertically since that is required...

Add flag to decoder class to flip the image vertically since that is required
on some platforms.

BUG=none
TEST=run code, verify that screen is upside-up.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51052

[dmac, 2010-06-28 20:41:50]

http://codereview.chromium.org/2878009/diff/5001/6001
File remoting/client/decoder_verbatim.cc (right):

http://codereview.chromium.org/2878009/diff/5001/6001#newcode13
remoting/client/decoder_verbatim.cc:13: reverse_rows_(true) {
how do we plan to set this per platform?

[awong, 2010-06-28 20:44:36]

I'm still not sure I understand why this is in the decoder...shouldn't the
encoding size normalize so the stream is consistent on the decode side?

-Albert


On Mon, Jun 28, 2010 at 1:41 PM, <dmaclach@chromium.org> wrote:

>
> http://codereview.chromium.org/2878009/diff/5001/6001
> File remoting/client/decoder_verbatim.cc (right):
>
> http://codereview.chromium.org/2878009/diff/5001/6001#newcode13
> remoting/client/decoder_verbatim.cc:13: reverse_rows_(true) {
> how do we plan to set this per platform?
>
>
> http://codereview.chromium.org/2878009/show
>

[garykac, 2010-06-28 21:05:17]

On 2010/06/28 20:44:36, awong wrote:
> I'm still not sure I understand why this is in the decoder...shouldn't the
> encoding size normalize so the stream is consistent on the decode side?

To support interop between all OSs, we need to be have code that can flip on
*both* the host and client sides:

If system A uses a lower-left coord system and system B uses a upper-left coord
system, then we simply need to choose one as the "network" coord system. 
Assuming we choose lower-left (to match A) for the "network":

A as host -> A as client : no flipping needed
A as host -> B as client : B (client) needs to flip
B as host -> A as client : B (host) needs to flip
B as host -> B as client : B (host & client) need to flip

So some systems will always have to flip (on both host/client), while others
will not.

There really isn't any cost to "flipping", it just means that we iterate from
the bottom instead of from the top.

[garykac, 2010-06-28 21:24:17]

Talking with dmac some more about this:

Since our assumption is that the client is less powerful than the host, we're
going to:
(1) select lower-left as the 'network origin'
(2) add a 'vflip' flag to the screen update packets (false = network origin;
true = upper-left)
(3) have the client tell the host whether it wants the image flipped during the
initial connection
(4) have the host flip the images (if needed) before sending them

Once we do this, the client will not need to flip the image at all. An exception
to this would be for screen-casts, where (unless all the clients agreed) some
clients would need to flip.

In any case, we need to add some host & protocol changes before we can do this.

[garykac, 2010-06-28 21:27:17]

On 2010/06/28 20:41:50, dmac wrote:
> http://codereview.chromium.org/2878009/diff/5001/6001
> File remoting/client/decoder_verbatim.cc (right):
> 
> http://codereview.chromium.org/2878009/diff/5001/6001#newcode13
> remoting/client/decoder_verbatim.cc:13: reverse_rows_(true) {
> how do we plan to set this per platform?

We need to expand the protocol so that each packet has a 'vflip' flag to know
when to flip.

I can add this later once you've checked in your chunk of code.  Right now, this
is a temp fix that works for the host-client combinations we're playing with.

[dmac, 2010-06-28 21:47:15]

On 2010/06/28 21:27:17, garykac wrote:
> On 2010/06/28 20:41:50, dmac wrote:
> > http://codereview.chromium.org/2878009/diff/5001/6001
> > File remoting/client/decoder_verbatim.cc (right):
> > 
> > http://codereview.chromium.org/2878009/diff/5001/6001#newcode13
> > remoting/client/decoder_verbatim.cc:13: reverse_rows_(true) {
> > how do we plan to set this per platform?
> 
> We need to expand the protocol so that each packet has a 'vflip' flag to know
> when to flip.
> 
> I can add this later once you've checked in your chunk of code.  Right now,
this
> is a temp fix that works for the host-client combinations we're playing with.

LGTM