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

Select media device for video and audio input #1450

Closed
wants to merge 9 commits into from

Conversation

Peterede
Copy link
Contributor

The user should be able to select the microphone and video input device as well as the audio output device. This pull request is for the issue #358

@fancycode
Copy link
Member

Please rebase the merge request and remove unrelated commits from #1422 from this branch.

@Peterede Peterede force-pushed the select_media_device branch from 5a3236e to c05c791 Compare January 21, 2019 22:29
@Peterede
Copy link
Contributor Author

rebased and removed unrelated commits

@@ -91,6 +111,7 @@
canFullModerate: this._canFullModerate(),
isPublic: this.model.get('type') === 3,
showShareLink: !canModerate && this.model.get('type') === 3,
canPublish: OCA.SpreedMe.canPublish(),
Copy link
Member

Choose a reason for hiding this comment

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

TypeError: OCA.SpreedMe.canPublish is not a function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from another pull request #1298

js/app.js Outdated Show resolved Hide resolved
js/app.js Show resolved Hide resolved
OCA.SpreedMe.webrtc.startLocalVideo(OCA.SpreedMe.webrtc.config.media);
OCA.SpreedMe.app.connection.joinCall(this.activeRoom.get('token'));

/*var senders = existingPeer.pc.getLocalStreams();
Copy link
Member

Choose a reason for hiding this comment

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

If the code is not needed remove it.

js/views/callinfoview.js Outdated Show resolved Hide resolved
css/style.scss Outdated Show resolved Hide resolved
css/style.scss Outdated Show resolved Hide resolved
css/style.scss Show resolved Hide resolved

.button {
cursor: pointer;
width: 50px;
Copy link
Member

Choose a reason for hiding this comment

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

should have the same size as all other icons

' <span class="button icon-settings"></span>' +
'</div>' +
'</div>' +
'<div class="settings-menu hidden">' +
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this should be in the callinfo-section, as it is not specific to the conversation, but in general?

Maybe it's time for us to start using the settings feature in the left sidebar. However I'm not sure we should go through the trouble, when we can do this easily after switching to Vue.JS ( #1347 )

Copy link
Contributor Author

@Peterede Peterede Jan 22, 2019

Choose a reason for hiding this comment

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

I think it is specific to the conversation just like the 'join call' button, the settings are used directly when joining the call and affect how the call appears and sounds. Putting the settings in the left sidebar might make them less obvious (and also I force the left sidebar to be hidden), the more exposed the camera, microphone and speaker settings are the better as this affects the user experience. If when the user connects their microphone is not working or their video is not working then the camera and microphone settings should be the first thing they check.

@nickvergessen nickvergessen added this to the 💚 Next Major milestone Jan 30, 2019
@nickvergessen
Copy link
Member

Closing as per #1450 (comment)

@djohle
Copy link

djohle commented Mar 30, 2022

I'm curious what happened to the "as well as the audio output device" portion of this...?

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

Successfully merging this pull request may close these issues.

4 participants