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

Feature/video streaming #345

Merged
merged 35 commits into from
Feb 12, 2021
Merged

Feature/video streaming #345

merged 35 commits into from
Feb 12, 2021

Conversation

iCollin
Copy link
Collaborator

@iCollin iCollin commented Jan 12, 2021

Adds Video Streaming
Fixes #38
Fixes #290
Fixes #315

@iCollin iCollin linked an issue Jan 13, 2021 that may be closed by this pull request
@@ -174,12 +178,13 @@ def __init__(self, _websocket, _path):
"InstallApp": RPCService.webengine_manager.handle_install_app,
"GetInstalledApps": RPCService.webengine_manager.handle_get_installed_apps,
"UninstallApp": RPCService.webengine_manager.handle_uninstall_app,
"StartVideoStream": self.handle_start_video_stream,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there needs to be a message added for Stopping a video stream.

src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
tools/start_server.py Outdated Show resolved Hide resolved

if (this.webmSupport === undefined) {
var video = document.getElementById('navi_stream');
this.webmSupport = 'probably' === video.canPlayType('video/webm; codecs="vp8"');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This returns 'probably'?

Copy link
Collaborator Author

@iCollin iCollin Feb 11, 2021

Choose a reason for hiding this comment

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

probably, maybe, or an empty string if it knows it does not support

it can never be sure cause there are still like bitrate limits etc

tools/start_server.py Outdated Show resolved Hide resolved

elif not _params['webm']:
print('\033[33mYour browser does not support WEBM video\033[0m')
if _params['config']['protocol'] == 'RAW' and _params['config']['protocol'] == 'H264':
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if _params['config']['protocol'] == 'RAW' and _params['config']['protocol'] == 'H264':
if _params['config']['protocol'] == 'RAW' and _params['config']['codec'] == 'H264':

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

applied in ba8ca35

if 'config' not in _params:
return self.gen_error_msg('Missing mandatory param \'config\'')

if _params['config']['protocol'] == 'RTP':
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend to use get() instead of direct map access because all config parameters are not mandatory. Python will throw the exception if you try to access key which does not exist

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good idea, non mandatory access replaced with get in ba8ca35

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