addyosmani / getUserMedia.js

Shim for getUserMedia(). Uses native implementation for modern browsers and a Flash fallback for everyone else.
MIT License
903 stars 158 forks source link

complete logic replacement of the object vs string style options #11

Closed franzenzenhofer closed 12 years ago

franzenzenhofer commented 12 years ago

as discussed in issue #10 this new logic just creates two configs, one as an object, the other one as a string. the upside is the there is now just one allowbar (issue +10 fixed) and #7 is fixed, too.

tested it i google chrome 21.0.1180.79 and 23.0.1237.0 canary (all max os x lion), works so far, not tested on other browsers or plattforms.

currently the fix is only in the /lib/getUserMedia.js, not in the /dist/ as i'm not quite sure how your build process works.

addyosmani commented 12 years ago

@franzenzenhofer This is so rad. Thanks for taking the time to work on the logic replacement. Browsers I'll be testing this against/which we need to have compat with:

addyosmani commented 12 years ago

Works in Chrome, Opera 12. Checking FF and IE8 onwards.

addyosmani commented 12 years ago

Close! We appear to break in the Firefox nightlies (no console errors, so I'm assuming its config related). If you'd like to test in the nightlies, it just takes a minute to enable getUserMedia support. Documented here http://www.browsomatic.com/2012/07/firefox-16-now-supports-html5.html.

franzenzenhofer commented 12 years ago

hi, good news everyone, the detection mechanism works as it should. but other stuff doesn't.

as i'm touching quite a lot of code areas and i still have lots of debug statements in there, so i write down merge instructions instead.

  1. this pull request above works as it should https://github.com/franzenzenhofer/getUserMedia.js/commit/e20a3bf72f680988de5a312662a0fdb84f00f297

but to make the face detection demo work on firefox nightly you have to do some changes (see here for a summary of the changes https://github.com/franzenzenhofer/getUserMedia.js/commit/2010d92ccacc09f665cac980ef2e4b9d7d197e58#lib/getUserMedia.js )

  1. https://github.com/franzenzenhofer/getUserMedia.js/blob/master/face-detection-demo/index.html#L54 i changed the path to use the /lib/ version (where i made the changes)
  2. https://github.com/franzenzenhofer/getUserMedia.js/blob/master/face-detection-demo/js/demo.js#L144-145 change the error handler to give you some information about what error happened
  3. turn of every other instance of webrtc in firefox (other than the one you are testing), otherwise you will get an HARDWARE_UNAVAILABLE error in the error handler
  4. https://github.com/franzenzenhofer/getUserMedia.js/blob/master/face-detection-demo/js/demo.js#L69 set audio to 'false' otherwise you will get a NOT_IMPLEMENTED error (?? this should probably be investigated, is audio in general not supported or audio, video together?)
  5. now for the big part https://github.com/franzenzenhofer/getUserMedia.js/blob/master/face-detection-demo/js/demo.js#L120-129 FF returns a MediaStream object, ok, we all know that but also, FF has de-prefixed window.URL so that's why this check var vendorURL = window.URL || window.webkitURL; will not help any longer, plus to actually make the video move, we need to call video.play() afterwards anyway, so this is enough justification to make an "if" statement.

so basically i check if stream is a MediaStream object, if it is, do the FF stuff, otherwise go with your logic.

that's it.

p.s.: i'm at Fronteers Conf in oct, looking forward to your talk.

addyosmani commented 12 years ago

Looking forward to seeing you at Fronteers :)

Thanks a lot for the detailed log of changes (and your work on getting FF compatibility in there), @franzenzenhofer. I'll make the updates you've listed today, test them and hopefully bring in for a merge. Will also have to credit you in the README for this lovely set of updates.

addyosmani commented 12 years ago

@franzenzenhofer I've just tested all of your changes in the fork, pulled them into master, cleaned them up to pass jshint (the build process can be called by running grunt at the command-line after its been installed npm install grunt -g and re-tested. It all works great across the browsers the project seeks to support :).

This includes Firefox stable, the nightlies and so on. Thank you so much for helping with this. As promised I've added you to the credits section of the site https://github.com/addyosmani/getUserMedia.js.

If there's anything that needs to be changed there, please let me know and I'll be happy to update. I've also pinged you on another thread about suggested API improvements.