Re: [Qemu-devel] [PATCHv4 6/6] ui/vnc: disable adaptive update calculations if not needed

[Peter Lieven]

On 13.01.2014 03:42, Wenchao Xia wrote:
> 于 2014/1/11 6:28, Peter Lieven 写道:
> > Am 10.01.2014 04:09, schrieb Wenchao Xia:
> > > 于 2014/1/10 0:25, Peter Lieven 写道:
> > > > Am 09.01.2014 09:29, schrieb Wenchao Xia:
> > > > > 于 2014/1/8 17:08, Peter Lieven 写道:
> > > > > > Signed-off-by: Peter Lieven <address@hidden>
> > > > > > ---
> > > > > >      ui/vnc.c |    9 +++++++++
> > > > > >      1 file changed, 9 insertions(+)
> > > > > >
> > > > > > diff --git a/ui/vnc.c b/ui/vnc.c
> > > > > > index da552fe..a742d32 100644
> > > > > > --- a/ui/vnc.c
> > > > > > +++ b/ui/vnc.c
> > > > > > @@ -3170,7 +3170,9 @@ void vnc_display_open(DisplayState *ds, const char 
> > > > > > *display, Error **errp)
> > > > > >                  acl = 1;
> > > > > >      #endif
> > > > > >              } else if (strncmp(options, "lossy", 5) == 0) {
> > > > > > +#ifdef CONFIG_VNC_JPEG
> > > > > >                  vs->lossy = true;
> > > > > > +#endif
> > > > > >              } else if (strncmp(options, "non-adaptive", 12) == 0) {
> > > > > >                  vs->non_adaptive = true;
> > > > > >              } else if (strncmp(options, "share=", 6) == 0) {
> > > > > > @@ -3187,6 +3189,13 @@ void vnc_display_open(DisplayState *ds, const char 
> > > > > > *display, Error **errp)
> > > > > >              }
> > > > > >          }
> > > > > >
> > > > > > +    /* adaptive updates are only used with tight encoding and
> > > > > > +     * if lossy updates are enabled so we can disable all the
> > > > > > +     * calculations otherwise */
> > > > > > +    if (!vs->lossy) {
> > > > > > +        vs->non_adaptive = true;
> > > > > > +    }
> > > > > > +
> > > > >      The code seems: if vs->loosy == false, then vs->non_adaptive = true,
> > > > > translate as: if loosy update is not used, then don't do adaptive
> > > > > update., which doesn't conform with the comments. I am not sure if this
> > > > > is on expectation.
> > > > It don't see the logic break. The option means non_adaptive, not adaptive.
> > > >
> > > > I write "adaptive updates are only used ... with lossy updates...". Which
> > >     So tight encoding means loosy updates?
> > It means you can only enable lossy updates if you have tight encoding. So if 
> > you are
> > missing tight encoding or lossy is false then you can set non_adaptive to true.
> >
> > Peter
>    I see the logic, guess I punctuated the comments in a wrong way, the
> real meaning may be:
>
>      /* Adaptive updates are only used with tight encoding and
>       * if lossy updates are enabled, so we can disable all the
>       * calculations otherwise */

Agreed. The comma there makes it clearer.

Peter


--

Mit freundlichen Grüßen

Peter Lieven

...........................................................

  KAMP Netzwerkdienste GmbH
  Vestische Str. 89-91 | 46117 Oberhausen
  Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
  address@hidden | http://www.kamp.de

  Geschäftsführer: Heiner Lante | Michael Lante
  Amtsgericht Duisburg | HRB Nr. 12154
  USt-Id-Nr.: DE 120607556

...........................................................