FURY Streaming System Proposal #437
Conversation
related with fury-gl#435
|
Hello @devmessias! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2022-04-13 14:53:03 UTC |
Co-authored-by: devmessias <messias.physics@gmail.com> Co-authored-by: filipinascimento <filipinascimento@gmail.com>
Doing that way because the FuryStreamClient will have a lot of more behaviors, like user interaction.
This can be used for example to store/read user interactions
This uses a circular queue as proposed by @filipinascimento
It seems that vtk uses left-bottom coordinates
macOS dosen't use fork to achieve multiprocessing. Therefore, this create some issues for specific python versions. The solution it's to pass just the buffers to the childreen process
For example, using ngrok ./ngrok http --subdomain=something 8000
Fix zoom factor for throttling
iframe mode, background color and interaction can be defined through url parameters Example: http://localhost:8000/?interaction=1&iframe=0&background=black
…uffers This allows you to start a webrtc server in the main process
This will avoid the first rendering to be just a blank screen inside the browser
devmessias
force-pushed
the
feature_fury_stream_client
branch
from
cff02af
to
eb85624
Compare
| Linux | ||
|
|
||
|
|
||
| `apt install libavdevice-dev libavfilter-dev libopus-dev libvpx-dev pkg-config` |
There was a problem hiding this comment.
I do not see ffmpeg here. Is that okay?
There was a problem hiding this comment.
Ok, I forgot. I'll address that
There was a problem hiding this comment.
it seems that for mac, ffmpeg, opus or libvpx are not needed. I've just pipinstall aiortc
There was a problem hiding this comment.
it seems that for mac, ffmpeg, opus or libvpx are not needed. I've just pipinstall aiortc
| None, | ||
| stream.img_manager.info_buffer, | ||
| None, | ||
| stream_interaction.circular_queue.head_tail_buffer, |
There was a problem hiding this comment.
The args can be simplified very much by just providing as input stream and stream_interaction. Then you can grab the other properties from the inside your function.
There was a problem hiding this comment.
This will not work on MacOs. Unfortunately, MacOs seems to have some problems sending a pickle to a child process
There was a problem hiding this comment.
Otherwise, create two classes one for shared memory and one for raw array.
| self.ms_jpeg = ms_jpeg | ||
| self._host = host | ||
| if port is None: | ||
| port = np.random.randint(7000, 8888) |
There was a problem hiding this comment.
Yes, I think so. @filipinascimento do you have any considerations?
There was a problem hiding this comment.
This do not check for existing open ports, it is safe, but can fail if the selected port is already open.
There was a problem hiding this comment.
We can check this. But this operation takes some time.
There was a problem hiding this comment.
Hi @devmessias,
See below for some comments, I still need to spend time on fury.stream.tools
to improve readability
To improve the code readability and design
devmessias
force-pushed
the
feature_fury_stream_client
branch
from
e1390da
to
051d42f
Compare
fury/stream/tools.py
Outdated
| def fix_register(name, rtype): | ||
| if rtype == "shared_memory": | ||
| return | ||
| return resource_tracker._resource_tracker.register( |
There was a problem hiding this comment.
self is undefined here. Results in a silent error when the server is running in another process.
There was a problem hiding this comment.
This is how we can do monkey patching on python. This is an open bug in CPython implementation regarding shared memory
There was a problem hiding this comment.
Alto this is a silent ERROR or silent Warning? What is the msg?
There was a problem hiding this comment.
Indeed this results in a warning caused by an unraisable exception, which I'm not sure if it is unraisable because it is running during garbage collector, destroy or because it runs in another process.
There was a problem hiding this comment.
What you can do is call ls -ls /dev/shm and see if some resource was not properly released.
There was a problem hiding this comment.
Ok, I've tested this, and the patch works on mac. The problem is that, for some reason, pytest checks this self and gives an NameError. On macs, /dev/shm does not exist, but I've checked for memory usage and everything seems to work nicely on Python 3.8.
There was a problem hiding this comment.
Adding a NameError try catch, as you suggested, gets rid of the warning for when running pytest. Memory is reclaimed. so everything should be fine. The problem does not happen on Windows.
There was a problem hiding this comment.
Done at 25b3979
Maybe is good to describe this issue on cypthon repo. python/cpython#82300
|
Also, do not forget to address all the pep8 listed above |
There was a problem hiding this comment.
Hi @devmessias Please see comments and also address pep 8 issues. Keep pushing this is getting super close to be merged! FURY streaming is on the way! Go go team!
It dosen't provides real improvements
python bug related to python/cpython#82300
devmessias
force-pushed
the
feature_fury_stream_client
branch
from
4a8f49f
to
25b3979
Compare
devmessias
force-pushed
the
feature_fury_stream_client
branch
from
b58807f
to
3fbaf24
Compare
|
Congratulations @devmessias This important PR is finally merged. Great job! |
|
Great news! Thanks @Garyfallidis , @skoudoro and @filipinascimento |
|
Next step @devmessias is the creation of furyspeed package that will accelerate streaming speed @skoudoro |
related with #435