Skip to content
This repository was archived by the owner on May 13, 2024. It is now read-only.

Conversation

fippo
Copy link
Contributor

@fippo fippo commented Jun 4, 2015

Lets dogfood on that promises stuff...

I found a really odd bug... createAnswers gets a third constraints argument which doesn't make sense.
@alvestrand is that worth bailing out on in adapter?

Copy link
Contributor

Choose a reason for hiding this comment

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

The answer constraints should not be removed, webrtc looks for it to decide if it receives media or uses bundle:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjingle/source/talk/app/webrtc/mediastreamsignaling.cc&rcl=1437050479&l=442

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hrm... that should hopefully work with https://github.com/webrtc/adapter/blob/master/adapter.js#L228-229
now if firefox doesn't error i can fix that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Firefox throws an error unfortunately :-(

@fippo
Copy link
Contributor Author

fippo commented Jul 16, 2015

@jiayliu worked around the issue -- ugly but shrug

I've also seen getStats failures since apprtc is using the legacy syntax without a browser switch but that's a different story (and also happens in the currently deployed version when opening the infobox)

@jiayliu
Copy link
Contributor

jiayliu commented Jul 16, 2015

Does it mean we have a regression in infobox on firefox?

@fippo
Copy link
Contributor Author

fippo commented Jul 16, 2015

I doubt it's a regression. Firefox getStats has never worked without a track selector as its called in https://github.com/webrtc/apprtc/blob/master/src/web_app/js/peerconnectionclient.js#L172

@jiayliu
Copy link
Contributor

jiayliu commented Jul 16, 2015

lgtm.
ok to merge?

@fippo
Copy link
Contributor Author

fippo commented Jul 16, 2015

yeah. The Firefox issue should soon be fixed but I guess it will take time to go to the stable channel there.

jiayliu added a commit that referenced this pull request Jul 16, 2015
Use promises with PeerConnection API
@jiayliu jiayliu merged commit 896373b into webrtc:master Jul 16, 2015
@fippo fippo deleted the moar-promises branch June 16, 2017 11:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants