On Mon, 27 Feb 2012 12:31:20 +0100
Arthur Huillet <arthur.huil...@free.fr> wrote:

> It sends a full update of the screen, in high quality JPEG format, after a 
> programmable idle time.
> This is disabled by default.
> ---
>  common/rfb/ServerCore.cxx       |    8 ++++++++
>  common/rfb/ServerCore.h         |    3 ++-
>  common/rfb/VNCSConnectionST.cxx |   29 ++++++++++++++++++++++++++++-
>  common/rfb/VNCSConnectionST.h   |    3 +++
>  4 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rfb/ServerCore.cxx b/common/rfb/ServerCore.cxx
> index ae2fd24..583648e 100644
> --- a/common/rfb/ServerCore.cxx
> +++ b/common/rfb/ServerCore.cxx
> @@ -93,4 +93,12 @@ rfb::BoolParameter rfb::Server::queryConnect
>  ("QueryConnect",
>   "Prompt the local user to accept or reject incoming connections.",
>   false);
> +rfb::BoolParameter rfb::Server::automaticLosslessRefresh
> +("AutomaticLosslessRefresh",
> + "Automatically refresh the framebuffer with lossless compression.",
> + false);
> +rfb::IntParameter rfb::Server::automaticLosslessRefreshDelay
> +("AutomaticLosslessRefreshDelay",
> + "Delay (in milliseconds) of inactivity after which to refresh framebuffer.",
> + 3000);
>  

Can't we combine these? E.g. let a delay of 0 mean disabled?

And I'm slightly put off by the length of the name for this thing. Can
we perhaps just drop the "Automatic" portion of the name? It should be
clear from the context anyway.

> diff --git a/common/rfb/ServerCore.h b/common/rfb/ServerCore.h
> index e12a8bc..745cdb5 100644
> --- a/common/rfb/ServerCore.h
> +++ b/common/rfb/ServerCore.h
> @@ -38,6 +38,7 @@ namespace rfb {
>      static IntParameter maxIdleTime;
>      static IntParameter clientWaitTimeMillis;
>      static IntParameter compareFB;
> +     static IntParameter automaticLosslessRefreshDelay;
>      static BoolParameter protocol3_3;
>      static BoolParameter alwaysShared;
>      static BoolParameter neverShared;

You have some indentation problems with your patch.

> diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
> index deec186..ec2e35e 100644
> --- a/common/rfb/VNCSConnectionST.cxx
> +++ b/common/rfb/VNCSConnectionST.cxx
> @@ -74,7 +74,7 @@ VNCSConnectionST::VNCSConnectionST(VNCServerST* server_, 
> network::Socket *s,
>      drawRenderedCursor(false), removeRenderedCursor(false),
>      continuousUpdates(false),
>      updateTimer(this), pointerEventTime(0),
> -    accessRights(AccessDefault), startTime(time(0))
> +    accessRights(AccessDefault), startTime(time(0)), alrTimer(this)
>  {
>    setStreams(&sock->inStream(), &sock->outStream());
>    peerEndpoint.buf = sock->getPeerEndpoint();

Similar naming peeve. Maybe refreshTimer?

> @@ -758,6 +758,10 @@ bool VNCSConnectionST::handleTimeout(Timer* t)
>        writeFramebufferUpdate();
>      else if (t == &congestionTimer)
>        updateCongestion();
> +     else if (t == &alrTimer) {
> +       fprintf(stderr, "Doing ALR\n");
> +       automaticLosslessRefresh();
> +      }
>    } catch (rdr::Exception& e) {
>      close(e.str());
>    }

Some debugging output still left in there. :)

> @@ -1108,6 +1114,8 @@ void VNCSConnectionST::writeFramebufferUpdate()
>      requested.clear();
>    }
>  
> +  if (rfb::Server::automaticLosslessRefresh)
> +    alrTimer.start(rfb::Server::automaticLosslessRefreshDelay);
>  out:
>    network::TcpSocket::cork(sock->getFd(), false);
>  }

This looks like the wrong place. You probably shouldn't start the timer
unless we actually sent a screen update.

Preferably you should also check that a lossy encoder was used, but
that's a bonus.

> @@ -1234,3 +1242,22 @@ int VNCSConnectionST::getStatus()
>    return 4;
>  }
>  
> +void VNCSConnectionST::automaticLosslessRefresh(void)
> +{
> +     // Automatic lossless refresh using JPEG Q95, 1X chroma sampling
> +     int q = cp.qualityLevel, fq = cp.fineQualityLevel;
> +     JPEG_SUBSAMP subsampling = cp.subsampling;
> +     cp.qualityLevel = 9;
> +     cp.fineQualityLevel = 95;
> +     cp.subsampling = SUBSAMP_NONE;
> +
> +     // Update all the screen (TODO: be smarter)
> +     framebufferUpdateRequest(Rect(0, 0, cp.width, cp.height), false);

It's probably safer to modify the update tracker directly.

Rgds
-- 
Pierre Ossman            OpenSource-based Thin Client Technology
System Developer         Telephone: +46-13-21 46 00
Cendio AB                Web: http://www.cendio.com

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: signature.asc
Description: PGP signature

------------------------------------------------------------------------------
Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-dev2
_______________________________________________
Tigervnc-devel mailing list
Tigervnc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-devel

Reply via email to