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

check for deviceId in optional[0].sourceId for adapterjs compatibility #283

Closed

Conversation

m59peacemaker
Copy link

Fixes #282

} else if (typeof constraints.video.sourceId === 'string') {
newConstraints.videoDeviceId = constraints.video.sourceId;
} else if (constraints.video.optional && constraints.video.optional[0] && typeof constraints.video.optional[0].sourceId === 'string') {
newConstraints.videoDeviceId = constraints.video.optional[0].sourceId;
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 this is too hacky. It will throw if constraints.video is undefined, and won't work if constraints.video.mandatory are given, or if sourceId appears in the second or third entry within optional.

Also, please split all those conditions in separate lines to make it more readable:

} else if (
	constraints.video &&
	(
		(constraints.video.optional &&
			constraints.video.optional[0] &&
			typeof constraints.video.optional[0].sourceId === 'string') ||
		(constraints.video.mandatory &&
			constraints.video.mandatory[0] &&
			typeof constraints.video.mandatory[0].sourceId === 'string')
	)
) {

@ibc
Copy link
Collaborator

ibc commented May 25, 2017

@saghul may you please participate here?

@m59peacemaker
Copy link
Author

m59peacemaker commented May 25, 2017 via email

@ibc
Copy link
Collaborator

ibc commented Dec 13, 2018

IMHO this should work without adapter.js stuff. It's adapter the one who is supposed to "adapt" stuff.

@hthetiot
Copy link
Contributor

hthetiot commented Aug 11, 2019

Using getUserMedia constraints.(video|audio).deviceId and getting values from cordova.plugins.iosrtc.getMediaDevices to get the deviceId and using webrtc-adapter v7.2.1+ works for me.

I think this PR can be closed unless I missed something.
cc @saghul

@hthetiot hthetiot added getUserMedia webrtc-adapter webrtc-adapter related labels Aug 14, 2019
@hthetiot
Copy link
Contributor

hthetiot commented Sep 10, 2019

Fixed partially by, but need more work to support mandatory and optional https://github.com/cordova-rtc/cordova-plugin-iosrtc/pull/374/files#diff-fca8d66f1ded96a88bd8bec01423cf9bR100

Will rebase PR soon for 5.0.2

@hthetiot hthetiot closed this Sep 10, 2019
@hthetiot hthetiot reopened this Sep 10, 2019
@hthetiot hthetiot closed this Sep 10, 2019
@hthetiot hthetiot reopened this Sep 10, 2019
@hthetiot hthetiot added this to the 5.0.x milestone Sep 10, 2019
@hthetiot hthetiot self-requested a review September 10, 2019 18:59
@hthetiot hthetiot self-assigned this Sep 10, 2019
@hthetiot hthetiot modified the milestones: 5.0.x, 5.0.3 Sep 13, 2019
@hthetiot
Copy link
Contributor

Closed in favor of #384

@hthetiot hthetiot closed this Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
getUserMedia webrtc-adapter webrtc-adapter related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webrtc/adapter breaks deviceId constraint
3 participants