Use Twisted for VLC communication #353
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rationale: asyncore/asynchat are deprecated since Python 3.6 and are going to be removed from the standard library from Python 3.10. It is unclear if these libraries will be picked up by maintainers and independently published on PyPI. At the moment, we are working on replacing them, in this commit with Twisted LineReceiver. Known issues: does not work with GUI. There is a conflict with qt5reactor -> "QSocketNotifier: Can only be used with threads started with QThread".
Sending the call to transport.write wrapped in a self.reactor.callFromThread instead of directly does the trick. Include also fixes to allow correct quit of VLC and Syncplay when either one is closed. Known issues: there is a noticeable lag (~ 1 second) between the start of Syncplay MainWindow and the start of VLC.
Member
albertosottile
left a comment
albertosottile
left a comment
There was a problem hiding this comment.
I remember writing this a year ago... I if I recall this correctly, preliminary tests on macOS worked fine, but this MR should be extensively tested on all the other platforms.
Also, there is this note in one of the commit messages that probably deserves an investigation:
Known issues: there is a noticeable lag (~ 1 second) between the start of Syncplay MainWindow and the start of VLC.
Contributor
Author
It's slightly faster than that for me on Windows - there is only usually a notable delay if I'm running VLC for the first time. |
Contributor
Author
|
Thanks for your work on this Alberto. |
albertosottile
added a commit
to albertosottile/syncplay
that referenced
this pull request
Sep 30, 2020
* Port VLCPlayer from asyncore/asynchat to Twisted Rationale: asyncore/asynchat are deprecated since Python 3.6 and are going to be removed from the standard library from Python 3.10. It is unclear if these libraries will be picked up by maintainers and independently published on PyPI. At the moment, we are working on replacing them, in this commit with Twisted LineReceiver. Known issues: does not work with GUI. There is a conflict with qt5reactor -> "QSocketNotifier: Can only be used with threads started with QThread". * Fix QSocketNotifier issue with qt5reactor Sending the call to transport.write wrapped in a self.reactor.callFromThread instead of directly does the trick. Include also fixes to allow correct quit of VLC and Syncplay when either one is closed. Known issues: there is a noticeable lag (~ 1 second) between the start of Syncplay MainWindow and the start of VLC. * Re-add try/except to VLC * Bring back missing try Co-authored-by: Alberto Sottile <[email protected]>
albertosottile
added a commit
to albertosottile/syncplay
that referenced
this pull request
Sep 30, 2020
* Port VLCPlayer from asyncore/asynchat to Twisted Rationale: asyncore/asynchat are deprecated since Python 3.6 and are going to be removed from the standard library from Python 3.10. It is unclear if these libraries will be picked up by maintainers and independently published on PyPI. At the moment, we are working on replacing them, in this commit with Twisted LineReceiver. Known issues: does not work with GUI. There is a conflict with qt5reactor -> "QSocketNotifier: Can only be used with threads started with QThread". * Fix QSocketNotifier issue with qt5reactor Sending the call to transport.write wrapped in a self.reactor.callFromThread instead of directly does the trick. Include also fixes to allow correct quit of VLC and Syncplay when either one is closed. Known issues: there is a noticeable lag (~ 1 second) between the start of Syncplay MainWindow and the start of VLC. * Re-add try/except to VLC * Bring back missing try Co-authored-by: Alberto Sottile <[email protected]>
Contributor
|
This may have broken VLC support for some, my friend coudn't use VLC with Syncplay v1.6.6 anymore. Syncplay would just simply not find the running VLC instance and it behaved as nothing was connected. Downgrading to v1.6.5 fixed it. Let me know if you would rather have me make a issue about it |
Contributor
Author
|
@smiba Thanks for bringing this to our attention. It would be helpful if you could create a new issue about this which includes the following information:
|
Contributor
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.