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

A function to clear video region detection, rather than setting to 0 coordinates and hoping for the best, might come in handy #1401

Closed
totaam opened this issue Jan 7, 2017 · 19 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Jan 7, 2017

Issue migrated from trac ticket # 1401

component: server | priority: major | resolution: fixed

2017-01-07 02:12:20: afarr created the issue


We're often detecting for video regions, but not quite certain of the best way to indicate to the server that we've no longer got video regions being detected, and that we thusly need the server to go back to using fresh heuristics, rather than continuing to 'wait and see' about those that might otherwise be 'up in the air'.

I'm assuming a control channel would be worthwhile, but a dbus call would be especially useful.

@totaam
Copy link
Collaborator Author

totaam commented Jan 7, 2017

2017-01-07 11:27:43: antoine changed owner from antoine to afarr

@totaam
Copy link
Collaborator Author

totaam commented Jan 7, 2017

2017-01-07 11:27:43: antoine changed title from A function to lear video region detection, rather than setting to 0 coordinates and hoping for the best, might come in handy to A function to clear video region detection, rather than setting to 0 coordinates and hoping for the best, might come in handy

@totaam
Copy link
Collaborator Author

totaam commented Jan 7, 2017

2017-01-07 11:27:43: antoine commented


Done in r14724 + r14727 fixup, you can reset the video region heuristics using:

  • xpra control :DISPLAY reset-video-region WINDOWID
  • via the dbus interface: ResetVideoRegion(WINDOWID)

This will reset everything except for the exclude regions list.

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 00:19:12: maxmylyn changed owner from afarr to antoine

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 00:19:12: maxmylyn commented


Did some testing with a trunk Fedora 25 r14735 server and client (two machines, same software versions):

Both the Control Channel and DBUS interfaces work with an important caveat -the control reset-video-region turns video detection back on whereas the DBUS equivalent does not.

I'll pass back to you to decide:

  • Leave it the way it is and make a note of it somewhere

or

  • Change either the DBUS or the Control Channel to give bring them up to parity.

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 05:59:06: antoine changed owner from antoine to afarr

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 05:59:06: antoine commented


They both turn video region detection back on since the code is the same: the dbus code (ResetVideoRegion in [/browser/xpra/trunk/src/xpra/server/dbus/dbus_server.py xpra/server/dbus/dbus_server.py]) calls the control interface code method (control_command_reset_video_region in [/browser/xpra/trunk/src/xpra/server/server_base.py xpra/server/server_base.py]).

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 17:11:01: maxmylyn commented


Okay that's weird because I compared both methods and using the DBUS didn't reset video regions, whereas using the control channel did.

For a testing method I connected using Firefox with the OpenGL paint boxes on and looped a video on YouTube - and using the control channels I set the video region to a small square on the top left of the window away from the video. When I reset it with the control channel, the heuristics found the video region immediately where it should be - and when I used the DBUS channel it didn't reset the video region.

I'll play around with it some more today.

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 19:49:41: maxmylyn changed owner from afarr to antoine

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 19:49:41: maxmylyn commented


Okay, I figured it out - the DBUS function you've provided asks for a Boolean rather than an integer for the $WID - following the server logs with -d dbus shows that the only values that get passed to the server are 0 or 1 regardless of what I give as input using D-Feet.

I'll attach a screenshot from D-Feet to show what I mean.

Here's a log snippet of me feeding 4,5,6 into D-Feet:

2017-01-10 11:49:04,636 org.xpra.Server.Get(server-idle-timeout)=0
2017-01-10 11:49:04,636 org.xpra.Server.Get(sharing)=False
2017-01-10 11:49:04,636 org.xpra.Server.Get(idle-timeout)=0
2017-01-10 11:49:04,636 org.xpra.Server.Get(name)=
2017-01-10 11:49:04,636 org.xpra.Server.GetAll(org.xpra.Server)={'server-idle-timeout': 0, 'sharing': False, 'idle-timeout': 0, 'name': ''}
2017-01-10 11:49:04,636 org.xpra.Server.ResetVideoRegion(1)
2017-01-10 11:49:07,412 org.xpra.Server.Get(server-idle-timeout)=0
2017-01-10 11:49:07,412 org.xpra.Server.Get(sharing)=False
2017-01-10 11:49:07,412 org.xpra.Server.Get(idle-timeout)=0
2017-01-10 11:49:07,412 org.xpra.Server.Get(name)=
2017-01-10 11:49:07,412 org.xpra.Server.GetAll(org.xpra.Server)={'server-idle-timeout': 0, 'sharing': False, 'idle-timeout': 0, 'name': ''}
2017-01-10 11:49:07,412 org.xpra.Server.ResetVideoRegion(1)
2017-01-10 11:49:09,484 org.xpra.Server.Get(server-idle-timeout)=0
2017-01-10 11:49:09,484 org.xpra.Server.Get(sharing)=False
2017-01-10 11:49:09,484 org.xpra.Server.Get(idle-timeout)=0
2017-01-10 11:49:09,484 org.xpra.Server.Get(name)=
2017-01-10 11:49:09,484 org.xpra.Server.GetAll(org.xpra.Server)={'server-idle-timeout': 0, 'sharing': False, 'idle-timeout': 0, 'name': ''}
2017-01-10 11:49:09,485 org.xpra.Server.ResetVideoRegion(1)

@totaam
Copy link
Collaborator Author

totaam commented Jan 10, 2017

2017-01-10 19:49:56: maxmylyn uploaded file 1401dfeet.png (32.9 KiB)

1401dfeet.png

@totaam
Copy link
Collaborator Author

totaam commented Jan 11, 2017

2017-01-11 03:42:24: antoine changed owner from antoine to maxmylyn

@totaam
Copy link
Collaborator Author

totaam commented Jan 11, 2017

2017-01-11 03:42:24: antoine commented


Excellent catch!
The windowid should always be a number, fixed in r14755.
(and obviously I only tested with a single window, id=1... which fits as a boolean)

@totaam
Copy link
Collaborator Author

totaam commented Jan 11, 2017

2017-01-11 17:52:27: maxmylyn changed owner from maxmylyn to antoine

@totaam
Copy link
Collaborator Author

totaam commented Jan 11, 2017

2017-01-11 17:52:27: maxmylyn commented


  • Upped server and client to r14763 trunk

The DBUS function works now!

I'll pass back to you with a request to backport this feature to 1.0.

@totaam
Copy link
Collaborator Author

totaam commented Jan 12, 2017

2017-01-12 02:41:41: antoine changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented Jan 12, 2017

2017-01-12 02:41:41: antoine set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented Jan 12, 2017

2017-01-12 02:41:41: antoine commented


I'll pass back to you with a request to backport this feature to 1.0.
[[BR]]
This isn't a bug fix so this isn't really suitable for 1.0, I have added this to the 1.1 queue: Versions PendingFixes

@totaam totaam closed this as completed Jan 12, 2017
@totaam
Copy link
Collaborator Author

totaam commented Dec 19, 2018

2018-12-19 18:45:51: antoine commented


See also #1060 for the original SetVideoRegionDetection and SetVideoRegionEnabled dbus api calls, and #1295 to exclude part of the window from the video region

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant