Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unset pixel buffer when x0vncserver client disconnects. #515

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 4 additions & 10 deletions common/rfb/VNCServerST.cxx
Expand Up @@ -313,6 +313,8 @@ void VNCServerST::setPixelBuffer(PixelBuffer* pb_, const ScreenSet& layout)
screenLayout = layout;

if (!pb) {
stopFrameClock();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it is better to do this when the desktop stops? I.e. in VNCServerST::removeSocket().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have placed it there because it is symetric with the startFrameClock() - it is called when pixel buffer is set, so stopFrameClock() is called when pixel buffer is unset.

Currently the call chain is VNCServerST::removeSocket() -> rfb::SDesktop::stop() (in this case x0vncserver's XDesktop::stop) -> VNCServerST::setPixelBuffer(0). But other implementation of rfb::SDesktop::stop may not want to unset the pixel buffer there, so the timer would stop and not start again for next connection? (Not sure.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, other server may not call VNCServerST::setPixelBuffer(0). However that won't wedge things as the clock will start again once new changes happen. The only downside is that the clock will continue to run for a bit after the desktop is "stopped", which might be a bit confusing. My idea was to make it more symmetrical with SDesktop::start()/stop().


if (desktopStarted)
throw Exception("setPixelBuffer: null PixelBuffer when desktopStarted?");
return;
Expand All @@ -337,18 +339,10 @@ void VNCServerST::setPixelBuffer(PixelBuffer* pb_, const ScreenSet& layout)

void VNCServerST::setPixelBuffer(PixelBuffer* pb_)
{
ScreenSet layout;

if (!pb_) {
if (desktopStarted)
throw Exception("setPixelBuffer: null PixelBuffer when desktopStarted?");
return;
}

layout = screenLayout;
ScreenSet layout = screenLayout;

// Check that the screen layout is still valid
if (!layout.validate(pb_->width(), pb_->height())) {
if (pb_ && !layout.validate(pb_->width(), pb_->height())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably reset the screen layout if we are removing the pixel buffer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll add that.

Rect fbRect;
ScreenSet::iterator iter, iter_next;

Expand Down