Reproducible with: - Firefox 35 RC (build2) - BuildID: 20150106233618 Reproducible on: Ubuntu 12.04 x86 NOT Reproducible on: Windows 7 x64, Mac OS X 10.9.5 Steps to reproduce: 1. Open Firefox and go to https://bug907352.bugzilla.mozilla.org/attachment.cgi?id=8418197. 2. Set Max values for Horizontal and Vertical at 1920. 3. Set Min values higher than 640x480 (e.g. 1200x700, 1024x768, 800x600). 4. Click the "Capture" button. Expected results: Video displays at the maximum camera capability (e.g. 640x480). Actual results: Video displays at 160x120. Notes: a) for Min values smaller than 640x480 the Video displays at 640x480 b) on Firefox 36 the camera sharing is blocked for Min values larger than 640x480 c) when Min values are 0, then the Video displays at the maximum values set for Max (up to the maximum camera resolution): - Max=640x480 or higher => 640x480 - Max=320x240 => 320x240 - Max=160x120 => 160x120

Could you do me a favor and test this on FF 32? I suspect this may be Bug 1119852 also.

(In reply to Jan-Ivar Bruaroey [:jib] from comment #1) > Could you do me a favor and test this on FF 32? I suspect this may be Bug > 1119852 also. This does not seem to be the same bug, as this issue also shows in Firefox 32. In fact it seems that this issue appeared in Firefox 31, as the regression window is: Last good = 2014-04-26 (0e91262606a6) First bad = 2014-04-27 (c67a79064fd4) https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0e91262606a6&tochange=c67a79064fd4 This seems consistent with implementation of width/height/framerate gUM constraints in bug 907352. In builds prior to this I get 640x480 for all values higher than 640x480.

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #0) > b) on Firefox 36 the camera sharing is blocked for Min values larger than 640x480 That's when we started respecting the require field, so that's correct behavior for this test. In 35- they're behaving like optional constraints effectively. The remaining problem in my view is how poorly they behave. The old spec algorithm is poor enough that if none of the constraints are met, then the resolution chosen is arbitrary (e.g. the first on the internal list of available modes). I expect this problem of poor matches to go away once I implement the new "ideal/exact" algorithm.

Clean up capabilities code before implementing ideal/exact algorithm. Specifically: gets rid of GuessCapability and uses a single algorithm. Requires hardcoding capabilities for OSX and B2G for now. Green try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d77f63ac75a

Comment on attachment 8555666 [details] [diff] [review] Part 1: streamline camera capabilities (remove alternate algorithm for OSX/B2G) Review of attachment 8555666 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp @@ +126,5 @@ > + { 540, 960, 15 }, > + { 480, 854, 15 }, > + { 480, 800, 15 }, > + { 320, 480, 15 }, > + }; You can get a list of the recorder profiles supported by the platform by calling GetRecorderProfiles() and GetProfileInfo(): https://dxr.mozilla.org/mozilla-central/source/dom/camera/ICameraControl.h#296 Also, you probably want to take whatever framerate you get and stuff that into GonkCameraSource::Create(): https://dxr.mozilla.org/mozilla-central/source/dom/media/webrtc/MediaEngineGonkVideoSource.cpp#228 @@ +127,5 @@ > + { 480, 854, 15 }, > + { 480, 800, 15 }, > + { 320, 480, 15 }, > + }; > + for (size_t i = 0; i < sizeof(hardcoded) / sizeof(*hardcoded); i++) { You can just use MOZ_ARRAY_LENGTH(hardcoded).

Comment on attachment 8555666 [details] [diff] [review] Part 1: streamline camera capabilities (remove alternate algorithm for OSX/B2G) Review of attachment 8555666 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp @@ +126,5 @@ > + { 540, 960, 15 }, > + { 480, 854, 15 }, > + { 480, 800, 15 }, > + { 320, 480, 15 }, > + }; I'm ok for me to hard code here temporary. Currently, components like media element, webrtc or media recorder uses gonk preview image when connecting to gUM, and the size of preview image is smaller than recording image in general. It's probably better to take it into consideration. And you should also add the 320x240 to list because it is the only one supported by emulator on try.

Incorporate feedback. Carrying forward r=ayang. Jesup, could you look at the desktop side? Green try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=65688f856328

I've removed the regression flag as I believe the "good" result of 640x480 in comment 0 is just the default result you'd get from constraints not doing anything at all.

(In reply to Mike Habicher [:mikeh] from comment #5) Thanks, I've filed Bug 1128550 as a follow-up.

Comment on attachment 8558072 [details] [diff] [review] Part 1: streamline camera capabilities (remove alternate algorithm for OSX/B2G) (2) r=ayang Review of attachment 8558072 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/webrtc/MediaEngineCameraVideoSource.cpp @@ +69,5 @@ > } > > +// Sub-classes (B2G or desktop) should overload one of both of these two methods > +// to provide capabilities > +int32_t in theory this should be size_t @@ +79,2 @@ > void > +MediaEngineCameraVideoSource::GetCapability(unsigned int aIndex, perhaps something other than "unsigned int"? @@ +107,5 @@ > +bool > +MediaEngineCameraVideoSource::SatisfiesConstraintSets( > + const nsTArray<const MediaTrackConstraintSet*>& aConstraintSets) > +{ > + int num = NumCapabilities(); not int, in any case. size_t @@ +110,5 @@ > +{ > + int num = NumCapabilities(); > + > + CapabilitySet candidateSet; > + for (int i = 0; i < num; i++) { ditto @@ +139,2 @@ > > + int num = NumCapabilities(); ditto @@ +141,3 @@ > > + CapabilitySet candidateSet; > + for (int i = 0; i < num; i++) { ditto @@ +145,5 @@ > + } > + > + // Pick among capabilities: First apply required constraints. > + > + for (uint32_t i = 0; i < candidateSet.Length();) { ditto-ish @@ +162,5 @@ > + > + if (aConstraints.mAdvanced.WasPassed()) { > + auto &array = aConstraints.mAdvanced.Value(); > + > + for (uint32_t i = 0; i < array.Length(); i++) { COnfusing that this is i ... j, and above it does something similar with j ... i @@ +164,5 @@ > + auto &array = aConstraints.mAdvanced.Value(); > + > + for (uint32_t i = 0; i < array.Length(); i++) { > + CapabilitySet rejects; > + for (uint32_t j = 0; j < candidateSet.Length();) { I'll stop mentioning this... ::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp @@ +126,5 @@ > + { 540, 960, 15 }, > + { 480, 854, 15 }, > + { 480, 800, 15 }, > + { 320, 480, 15 }, > + { 240, 320, 15 }, // sole mode supported by emulator on try Does this need to support landscape captures as well? I think so

Comment on attachment 8558072 [details] [diff] [review] Part 1: streamline camera capabilities (remove alternate algorithm for OSX/B2G) (2) r=ayang Review of attachment 8558072 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp @@ +135,5 @@ > + c.height = hardcoded[i].height; > + c.maxFPS = hardcoded[i].framerate; > + mHardcodedCapabilities.AppendElement(c); > + c.width = hardcoded[i].height; > + c.height = hardcoded[i].width; Landscape is here.

Getting patch off my laptop for service. Seems to work http://jsfiddle.net/jib1/Lrxwgeej Needs more tests and a bit of cleanup (isn't sorting cameras by best distance yet). Seeking feedback on new fitness algorithm mostly. Optimized for maintenance and adherence over speed.

Comment on attachment 8564727 [details] [diff] [review] Part 2: support ideal/exact constraint syntax WIP Review of attachment 8564727 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure about this one. Overall, the implementation of the constraints algorithm looks like an improvement, but I have structural concern. Looking at MediaEngineCameraVideoSource and MediaEngineTabVideoSource. They should be running a standardized algorithm, they each seem to be implementing the constraints algorithm independently. That's not going to work out well in the long run. I think that you probably need to think about building a generic processor for constraints, with individual classes for finding distances on each of the different axes. Then, each of the different sources can just configure which classes apply. Just spitballing, but width might look like: class WidthConstraintChecker : public ConstraintChecker { public: uint32_t GetDistance(const webrtc::CaptureCapability& aCandidate, const MediaTrackConstraintSet &aConstraints) const MOZ_OVERRIDE { return FitnessDistance(int32_t(aCandidate.width), aConstraints.mWidth); }; }; And then all you would need to do in each engine would be to identify what checkers are needed and add them to be used. I know that the tab thing can be simpler, but that sort of optimization is worth less than having a consistent implementation. Also, could the extra rounds you have to apply a preference for the defaults resolution or to apply a preference for a webrtc-compatible capture format be integrated by trial running another round of the core algorithm? ::: dom/media/webrtc/MediaEngineCameraVideoSource.cpp @@ +271,5 @@ > + GetCapability(candidateSet[i].mIndex, cap); > + if (cap.rawType == webrtc::RawVideoType::kVideoI420 || > + cap.rawType == webrtc::RawVideoType::kVideoYUY2 || > + cap.rawType == webrtc::RawVideoType::kVideoYV12) { > + mCapability = cap; This is going to pick the last candidate capability that has one of these formats. Why not roll the format stuff into the preference algorithm? ::: dom/media/webrtc/MediaEngineTabVideoSource.cpp @@ +122,5 @@ > + struct Range > + { > + ValueType mMin, mMax; > + dom::Optional<ValueType> mIdeal; > + ws

(In reply to Martin Thomson [:mt] from comment #13) This patch updates our existing screensharing and camera code - which already parse their respective constraints independently, as they don't overlap - to use spec-compliant syntax rather than the blip of last year's syntax-mistake that no-one else uses. I'd like us to consider landing this linear increment of progress ASAP as it removes a huge wort for 38 and ESR. Said differently, if we want to forget about the syntax-mistake as soon as we can, then making 38 seems imperative. Can we open a separate bug on transforming the two disparate uses into customers of a centralized general algorithm? There are real obstacles to a general algorithm, as discussed in [1] and some questions in my mind about the net win, given how different the back-ends are (camera-width and screensharing-width are largely namesakes). > class WidthConstraintChecker : public ConstraintChecker { > public: > uint32_t GetDistance(const webrtc::CaptureCapability& aCandidate, > const MediaTrackConstraintSet &aConstraints) const For instance, webrtc::CaptureCapability here is specific to cameras and how camera capabilities are stored (finite combination-list). The very notion of aCandidate for an orthogonal value like Volume or, to some extent, screensharing-width would need rethinking as there are too many candidates to list (something like Harald's min-max-triplet suggested in [1] might work). I agree it's not great for each use of constraints to have to re-implement the algorithm, but in large parts there's an amount of work here regardless, considering that there is hardly any overlap in constraints. Doing that work requires at minimum an understanding of the significance of fitness distance, which doesn't seem that much more involved than knowing the for-loops to crank the algorithm. Coming up with the right framework and/or helper classes that shield us from making mistakes in the future is going to be difficult, but worthwhile to work out, though ultimately we'll need to rely on tests, since the amount of actual code-reuse is marginal. FWIW, I want to move the "FlattenConstraints" helper class (for 100% orthogonal constraints only) to a central location, such that it can be reused. I just wrote it near its use for convenience. > Also, could the extra rounds you have to apply a preference for the defaults > resolution or to apply a preference for a webrtc-compatible capture format > be integrated by trial running another round of the core algorithm? Not following, tell me more. Also, wouldn't that require a rawVideoType constraint (not saying it's a bad idea)? > This is going to pick the last candidate capability that has one of these formats. Ah, not good. Thanks. [1] https://github.com/w3c/mediacapture-main/issues/118

Incorporate feedback on indexes. Made CapabilitySet index a size_t as well instead of uint8_t, to support more than 255 capabilities. Carrying forward r=ayang.

Just an unbitrot. Working on moving some of the code, otherwise looking to putting up for review something much like this, unless I hear objections.

Fixed some bugs, and moved some code. Helper classes are now in a central place rather than in MediaEngineTabVideoSource.cpp.

Comment on attachment 8565457 [details] [diff] [review] Part 1: streamline camera capabilities (remove alternate algorithm for OSX/B2G) (3) r=ayang Review of attachment 8565457 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp @@ +115,5 @@ > + // The camera-selecting constraints algorithm needs a set of capabilities to > + // work on. In lieu of something better, here are some generic values based on > + // http://en.wikipedia.org/wiki/Comparison_of_Firefox_OS_devices on Jan 2015. > + // When unknown, better overdo it with choices to not block legitimate asks. > + // TODO: Match with actual hardware or add code to query hardware. Open a bug and list it here. CC mikeh and mwu @@ +126,5 @@ > + { 540, 960, 15 }, > + { 480, 854, 15 }, > + { 480, 800, 15 }, > + { 320, 480, 15 }, > + { 240, 320, 15 }, // sole mode supported by emulator on try B2G cameras always support 30fps at some resolutions if not all. Flame certainly does. Check with mikeh/mwu and update as needed

From irc with mikeh, will add 15 and 30 framerates (I hear these seetings are really max framerates in practice, i.e. hardware may deliver less). Followup filed in comment 9.

Add 30fps to B2G hardcode list. Carrying forward r=ayang. Jesup, mind taking a last look? I updated for-loops to avoid i/j.

Comment on attachment 8566137 [details] [diff] [review] Part 1: streamline camera capabilities (remove alternate algorithm for OSX/B2G) (4) r=ayang Green try https://treeherder.mozilla.org/#/jobs?repo=try&revision=c34fc9b2afd6

Comment on attachment 8565836 [details] [diff] [review] Part 2: support ideal/exact constraint syntax (3) May end up taking first review I can get.

Comment on attachment 8565836 [details] [diff] [review] Part 2: support ideal/exact constraint syntax (3) Green try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=d63bd277418b

With patches from Bug 1134112 in place, tests now run, which exposed that two of them needed fixing. This should fare better. Try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=13de159d8316

Part 1 can be landed.

Comment on attachment 8566322 [details] [diff] [review] Part 2: support ideal/exact constraint syntax (4) Review of attachment 8566322 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/tests/mochitest/test_getUserMedia_constraints.html @@ +15,5 @@ > */ > var tests = [ > // Each test here tests a different constraint or codepath. > + { message: "unknown required constraint on video ignored", > + constraints: { video: { somethingUnknown:{ exact:0 } }, nit: inconsistent spacing after colons here and below ::: dom/media/webrtc/MediaEngineCameraVideoSource.cpp @@ +262,5 @@ > + // Any remaining multiples all have the same distance, but may vary on > + // format. Some formats are more desirable for certain use like WebRTC. > + // E.g. I420 over RGB24 can remove a needless format conversion. > + > + GetCapability(candidateSet[0].mIndex, mCapability); replace with: bool found = false; @@ +264,5 @@ > + // E.g. I420 over RGB24 can remove a needless format conversion. > + > + GetCapability(candidateSet[0].mIndex, mCapability); > + > + for (size_t i = 1; i < candidateSet.Length(); i++) { Start at 0. @@ +270,5 @@ > + GetCapability(candidateSet[i].mIndex, cap); > + if (cap.rawType == webrtc::RawVideoType::kVideoI420 || > + cap.rawType == webrtc::RawVideoType::kVideoYUY2 || > + cap.rawType == webrtc::RawVideoType::kVideoYV12) { > + mCapability = cap; found = true; break; @@ +275,2 @@ > } > } if (!found) { GetCapability(candidateSet[0].mIndex, mCapability); } ::: dom/media/webrtc/MediaEngineTabVideoSource.cpp @@ +5,5 @@ > > #include "MediaEngineTabVideoSource.h" > > + > +#include <limits> Maybe move this to where it is actually used. @@ +135,5 @@ > + mBufHeightMax = c.mHeight.Clamp(c.mHeight.mIdeal.WasPassed() ? > + c.mHeight.mIdeal.Value() : aPrefs.GetHeight(false)); > + double frames = c.mFrameRate.Clamp(c.mFrameRate.mIdeal.WasPassed() ? > + c.mFrameRate.mIdeal.Value() : aPrefs.mFPS); > + mTimePerFrame = frames ? 1000 / int(frames) : 0; s/frames/frameRate/ What prevents this from being negative? Also, if the frame rate is set to 0, can we drop to 1 frame per second instead (i.e., 1000). I checked nsITimer and it looks like setting this to zero will generate a tight loop. We probably need some sort of floor on this too (10ms perhaps) to prevent a similar effect by having a frame rate of 1e20. Also, I'd do the cast to int after the division, which would allow someone to express a need for a frame time that exceeds 1s, or one that isn't near an integral divisor of 1000. ::: dom/media/webrtc/MediaTrackConstraints.cpp @@ +44,5 @@ > +} > + > +NormalizedConstraintSet::DoubleRange::DoubleRange( > + const dom::OwningDoubleOrConstrainDoubleRange& aOther, bool advanced) > +: Range<double>(-std::numeric_limits<double>::infinity(), I think that I'd like to see #include <limits> in this file or its header. ::: dom/media/webrtc/MediaTrackConstraints.h @@ +82,5 @@ > + struct AdvancedConstraintSet : public NormalizedConstraintSet > + { > + AdvancedConstraintSet(const dom::MediaTrackConstraintSet& aOther) > + : NormalizedConstraintSet(aOther, true) {} > + }; I'd drop AdvancedConstraintSet in favour of just using the normalized set directly.

Incorporate feedback. Carrying forward r=mt.

webidl changes need dom peer review

Comment on attachment 8567109 [details] [diff] [review] wrong file This patch is just the Try patch...

Ugh, moving too fast this morning, sorry. Real patch with incorporated feedback. Carrying forward r=jesup. Requesting DOM review.

Green try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=b38445c731ee

Update commit msg. Thanks!

https://hg.mozilla.org/mozilla-central/rev/a1463070ce71 https://hg.mozilla.org/mozilla-central/rev/e9a8fe34405c https://hg.mozilla.org/mozilla-central/rev/617202816856 https://hg.mozilla.org/mozilla-central/rev/0b9a16b993c9

I was able to reproduce this issue on Firefox 35 RC (build2) - BuildID: 20150106233618 using Ubuntu 14.04 32bit. I tested this bug on Firefox 38.0a2 (2015-03-19) and Firefox 39.0a1 (2015-03-18) using Ubuntu 14.04 32bit and I noticed that the webcam does not connect anymore (the share doorhanger is not displayed) if Min values higher than 640x480 are selected (e.g. 1200x700, 1024x768, 800x600). Is this the expected behavior?

It depends entirely on what your camera supports. If you request more than it can handle then failing is correct. Please look in web console for the correct spec errors when it fails. The hardware factor is why we don't have better automated tests, and why manual testing is imperative. More importantly, these patches implement a new feature - constraints to the specification, including the ideal algorithm, which we didn't have before - which goes beyond the symptoms filed in this bug. In other words: this bug adds functionality on all the platforms, that require more comprehensive testing. Should you find the spec hard to follow (see URL), then I encourage you to look at this answer [1] for a higher level explanation of what to expect as far as behavior. Also note that Google Chrome and Opera don't implement this yet (they use an old outdated syntax and algorithm), so comparing for parity wont work here. The best parity test would be to look at my pull request for adapter.js [2], the polyfill meant to bridge browser-differences, where I convert spec constraints to browser-specific legacy formats for all browsers *except* Firefox 38 and newer (thanks to this patch). The PR also modifies two of the samples which can be used for parity testing with other browsers. Lastly, to experiment with different constraints, some jsfiddles that I've used may be helpful [3][4][5]. Feel free to modify them. Thanks. [1] http://stackoverflow.com/questions/28282385/webrtc-firefox-constraints/28911694#28911694 [2] https://github.com/webrtc/samples/pull/493 [3] Basic resolution tester http://jsfiddle.net/0n4aqucj [4] Android facingMode http://jsfiddle.net/ee9cct6b [5] OSX mass enumerator http://jsfiddle.net/35p15x4c

I got [4] and [5] mixed up. You get the idea.

Today, I tested with a Microsoft Lifecam 3000 hd webcam, which supports up to 720p = 1280x720. I followed the steps from comment 0: - Setting the Min values 1280x720 -> the share doorhanger appeared and the webcam was successfully connected, but the video was not displayed and no error was thrown in browser console. This happens for Min values higher than 640x480, even if my webcam supports more than that. - Setting the Min values smaller than 640x480, the video is displayed at 640x480. Is it right?

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #40) > I followed the steps from comment 0: > - Setting the Min values 1280x720 -> the share doorhanger appeared and the > webcam was successfully connected, but the video was not displayed and no > error was thrown in browser console. The script in comment 0 is old and has some timing bugs related to video.onloadedmetadata. Here's an updated version. I also updated it slightly: - To remove a constraint entirely set both values to 0. - Since the frameRate constraint may limit your choices when used, I've defaulted it to 0, 0. - Works on Chrome as well for parity testing (on OSX, not on Windows for some reason). > This happens for Min values higher than > 640x480, even if my webcam supports more than that. Please let me know if the new script helps. Also feel free to use some of the Firefox-specific jsfiddles in comment 38. Here's another one that displays frameRate: http://jsfiddle.net/h85ntyL5 > - Setting the Min values smaller than 640x480, the video is displayed at > 640x480. Is it right? Yes, because 640x480 is one of the two default resolutions in Firefox, the other being 1280x768 (it's a dynamic default). This dynamic default may be overridden in about:config by setting non-zero values for media.navigator.video.default_width and media.navigator.video.default_width. min and max define an allowed range for a value, so to force the browser to choose a resolution lower than the default you have to apply a max constraint.

(In reply to Jan-Ivar Bruaroey [:jib] from comment #42) > - Works on Chrome as well for parity testing (on OSX, not on Windows for some reason). Nevermind, works fine on Windows too.

If we believe we're setting the sizes correctly and we're still not getting 720p video from the camera, please try setting NSPR_LOG_MODULES=getusermedia:5,mediamanager:5 and NSPR_LOG_DEBUG=some_file and try the test, then attach the log file here.

I tested again with the same Microsoft Lifecam 3000 hd webcam, setting the Min Values 1280x720 and this are the results: 1.With http://jsfiddle.net/0n4aqucj -> Success: 352x288 2.With http://jsfiddle.net/h85ntyL5 -> Success: 640x480 3.With https://bug907352.bugzilla.mozilla.org/attachment.cgi?id=8418197 -> 352x288 I tested on Firefox 39.0a1 (2015-03-26) using Ubuntu 14.04 32bit. I am also attaching the log file. I hope it is ok.

Do you mean you modified the jsfiddles and examples to set Min values of 1280x720 in each of the three cases (including uncommenting width and height in the second one)? If so then the results are not ok as the minimum should be respected or the call must fail. To avoid confusion please fork the jsfiddles with the actual values you tested and link to them. We haven't done extensive testing on linux so it is possible that it is broken there. We'll probably also want more comprehensive testing here on several platforms, then we can isolate which platforms are working and which are not, and with which cameras. This might take different people with different setups, so I totally understand if you don't have several cameras on several systems.

Comment on attachment 8583867 [details] firefox.log Thanks, but we need a different log. Try setting these environment variables: NSPR_LOG_MODULES=getusermedia:5,mediamanager:5 NSPR_LOG_DEBUG=/tmp/nspr.log where the latter is where you want the log file to be placed.

I've performed some testing as well today using two webcams (but not the model that Vasilica used), and https://bugzilla.mozilla.org/attachment.cgi?id=8582572. Windows and Mac seem to work well, while on Linux the camera seems to be limited by the drivers it has and I also got some weird results for one camera. Testing was done on Windows 7 x64, Mac OS X 10.10 and Ubuntu 14.04 x64. See details below. Test page used: https://bugzilla.mozilla.org/attachment.cgi?id=8582572 Max value used (both horizontal and vertical): 1920 "xxxxxxx" (see below) means that the camera was not showing for selection in the sharing doorhanger Webcam = Logitech C920 Min= Win Mac Ubuntu 0x0 – 640x480 || 640x480 || 640x480 160x120 – 640x480 || 640x480 || 640x480 320x240 – 640x480 || 640x480 || 640x480 640x480 – 640x480 || 640x480 || 640x480 800x600 – 800x600 || 1152x648 || 960x720 -> limit 1024x576 – 1024x576 || 1024x576 || xxxxxxx 1024x768 – 1600x896 || 1408x792 || xxxxxxx 1200x700 – 1280x720 || 1280x720 || xxxxxxx 1280x720 – 1280x720 || 1280x720 || xxxxxxx 1280x960 – 1920x1080 || 1792x1008 || xxxxxxx 1290x970 – 1920x1080 || 1792x1008 || xxxxxxx 1920x1080 – 1920x1080 || 1920x1080 || xxxxxxx Results above seem to be the expected ones, with the exception of: - Ubuntu - the camera seems limited to 960x720 (I suspect it's likely because of the drivers) - Mac - some selected resolutions seem to be a bit non-standard, but still we support up to 1080p Webcam = Logitech Quickcam Fusion Min= Win Mac Ubuntu 0x0 – 1024x576 || xxxxxxx || 1024x576 160x120 – 1024x576 || xxxxxxx || 1024x576 320x240 – 1024x576 || xxxxxxx || 1024x576 640x480 – 1024x576 || xxxxxxx || 1024x576 800x600 – 960x720 || xxxxxxx || 960x720 1024x576 – 1024x576 || xxxxxxx || xxxxxxxx -> weird how 1024x576 is not selected 1024x768 – 1280x960 || xxxxxxx || xxxxxxxx 1200x700 – 1280x960 || xxxxxxx || xxxxxxxx 1280x720 – 1280x960 || xxxxxxx || xxxxxxxx 1280x960 – 1280x960 || xxxxxxx || xxxxxxxx 1290x970 – xxxxxxxx || xxxxxxx || xxxxxxxx Results above seem to be the expected ones, with the exception of: - Mac - webcam is not supported by the OS - Ubuntu - 1024x576 is selected for Min values smaller than 640x480, but NOT for Min values of exactly 1024x576, which seems very weird (see the log file attached below taken for Min values of 640x480, 800x600, 1024x576).

Log taken with Logitech Quickcam Fusion on Ubuntu 14.04 x64. Steps captured in the log: 1. Go to https://bugzilla.mozilla.org/attachment.cgi?id=8582572. 2. Set Max Horizontal = Max Vertical = 1920. 3. Set Min Horizontal=640 & Min Vertical=480, then click "Capture" -> 1024x576 selected for Video display 4. Set Min Horizontal=800 & Min Vertical=600, then click "Capture" -> 960x720 selected for Video display 5. Set Min Horizontal=1024 & Min Vertical=576, then click "Capture" -> NO option to select camera in the sharing doorhanger

Florin: what version of firefox? Did you have NSPR_LOG_MODULES=getusermedia:5,mediamanager:5 ? Usually I expect to see a series of capabilities listed first before one is chosen. Try adding webrtc_trace:65535 to NSPR_LOG_MODULES, and WEBRTC_TRACE_FILE=nspr Thanks!

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #48) > Max value used (both horizontal and vertical): 1920 Thanks, getting numbers from real cameras is helpful! Note though that by using min and max you're not actually testing the new ideal algorithm. ;-) E.g. you should be able to specify { width: 1024, height: 576 } directly now and get the best match. Please try this one: http://jsfiddle.net/dne7q71j

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #48) > 1024x576 – 1024x576 || xxxxxxx || xxxxxxxx -> weird how 1024x576 is not selected Yes that's not right. (In reply to Florin Mezei, QA (:FlorinMezei) from comment #49) > 4. Set Min Horizontal=800 & Min Vertical=600, then click "Capture" > -> 960x720 selected for Video display > 5. Set Min Horizontal=1024 & Min Vertical=576, then click "Capture" > -> NO option to select camera in the sharing doorhanger Are you refreshing the page between clicking "Capture"? No other tabs accessing the camera at the same time?

(In reply to Randell Jesup [:jesup] from comment #50) > Florin: what version of firefox? Did you have > NSPR_LOG_MODULES=getusermedia:5,mediamanager:5 ? This was with Firefox 38 RC build 3 (BuildID= 20150508094354), and the following in the terminal: export NSPR_LOG_MODULES=getusermedia:5,mediamanager:5 export NSPR_LOG_FILE=~/Desktop/logcam1.txt ./firefox > Try adding webrtc_trace:65535 to NSPR_LOG_MODULES, and WEBRTC_TRACE_FILE=nspr Attaching log (see below) for: export NSPR_LOG_MODULES=getusermedia:5,mediamanager:5,webrtc_trace:65535 export WEBRTC_TRACE_FILE=nspr export NSPR_LOG_FILE=~/Desktop/logcam2.txt ./firefox (In reply to Jan-Ivar Bruaroey [:jib] from comment #52) > Are you refreshing the page between clicking "Capture"? No other tabs > accessing the camera at the same time? I am not refreshing the page between clicking "Capture", and I don't think this has any influence since I still reproduce the issue on Ubuntu on a fresh profile when attempting 1024x576. Also, there are no other tabs or apps using the camera at the same time. > Please try this one: http://jsfiddle.net/dne7q71j Win Ubuntu {width:160,height:120} => 160x120 || 1024x576 {width:320,height:180} => 320x240 || 1024x576 {width:320,height:240} => 320x240 || 1024x576 {width:640,height:360} => 640x360 || 1024x576 {width:640,height:480} => 640x480 || 1024x576 {width:800,height:600} => 704x576 || 704x576 {width:1024,height:576} => 1024x576 || 960x720 {width:1024,height:768} => 960x720 || 960x720 (InternalError once after restart) {width:1200,height:700} => 960x720 || 960x720 (InternalError once after restart) {width:1280,height:960} => 1280x960 || 960x720 (InternalError once after restart) {width:1920,height:1080} => 1280x960 || 960x720 (InternalError once after restart) Let me know if you need further information from me.

Logs with Firefox 38 RC build 3 on Ubuntu 14.04 x64, and: export NSPR_LOG_MODULES=getusermedia:5,mediamanager:5,webrtc_trace:65535 export WEBRTC_TRACE_FILE=nspr export NSPR_LOG_FILE=~/Desktop/logcam2.txt ./firefox

For the attached log above I used the exact same scenario as in comment 49.

(In reply to Florin Mezei, QA (:FlorinMezei) [PTO - May 18-29] from comment #53) > {width:640,height:480} => 640x480 || 1024x576 > {width:800,height:600} => 704x576 || 704x576 > {width:1024,height:576} => 1024x576 || 960x720 Odd. Could you tell me the frame-rate you get for each of these using http://jsfiddle.net/vvrnc0pp ? > {width:1024,height:768} => 960x720 || 960x720 (InternalError once after > restart) Internal error was only on Ubunto I assume? Could you look in Tools / Web Developer / Browser Console (as opposed to Web Console) when this happens? There's sometimes more info there when internal errors happen. If you see any trace of this internal error in the logs that would be great also.

Lastly, if you could try it in Nightly as well I'd appreciate it. Sorry for all these directions, cameras are tough to debug.

(In reply to Jan-Ivar Bruaroey [:jib] from comment #56) > > {width:640,height:480} => 640x480 || 1024x576 > > {width:800,height:600} => 704x576 || 704x576 > > {width:1024,height:576} => 1024x576 || 960x720 > > Odd. Could you tell me the frame-rate you get for each of these using > http://jsfiddle.net/vvrnc0pp ? Firefox 38 RC build 3 Win Ubuntu {width:640,height:480} => 1024 x 576 x 10.01 || 960 x 720 x 15.09 {width:800,height:600} => 1024 x 576 x 10.01 || 960 x 720 x 15.01 {width:1024,height:576} => 1024 x 576 x 10.01 || 960 x 720 x 15.01 > > {width:1024,height:768} => 960x720 || 960x720 (InternalError once after > > restart) > > Internal error was only on Ubunto I assume? Yes, I only saw this once on Ubuntu. > Could you look in Tools / Web Developer / Browser Console (as opposed to Web > Console) when this happens? There's sometimes more info there when internal > errors happen. If you see any trace of this internal error in the logs that > would be great also. I tried to reproduce this error by restarting and repeating the test multiple times, but I did not get it anymore. (In reply to Jan-Ivar Bruaroey [:jib] from comment #57) > Lastly, if you could try it in Nightly as well I'd appreciate it. Sorry for > all these directions, cameras are tough to debug. I tried latest Nightly and it behaves exactly the same as Firefox 38 RC, on both Windows and Ubuntu: http://jsfiddle.net/dne7q71j Win Ubuntu {width:160,height:120} => 160x120 || 1024x576 {width:320,height:180} => 320x240 || 1024x576 {width:320,height:240} => 320x240 || 1024x576 {width:640,height:360} => 640x360 || 1024x576 {width:640,height:480} => 640x480 || 1024x576 {width:800,height:600} => 704x576 || 704x576 {width:1024,height:576} => 1024x576 || 960x720 {width:1024,height:768} => 960x720 || 960x720 {width:1200,height:700} => 960x720 || 960x720 {width:1280,height:960} => 1280x960 || 960x720 {width:1920,height:1080} => 1280x960 || 960x720 http://jsfiddle.net/vvrnc0pp Win Ubuntu {width:640,height:480} => 1024 x 576 x 10.01 || 960 x 720 x 15.01 {width:800,height:600} => 1024 x 576 x 10.01 || 960 x 720 x 15.01 {width:1024,height:576} => 1024 x 576 x 10.01 || 960 x 720 x 15.01 Note that I will be away for the next two weeks. If you need more information here please needinfo Andrei Vaida, and if there's time available then someone in the team will try to provide additional information.

Jan-Ivar, any follow-up on this issue... should we file a new bug to track the remaining issues?

Yes please.