Closed Bug 1094539 Opened 10 years ago Closed 10 years ago

Camera width/height/framerate constraints stopped working (REGRESSION)

Categories

(Core :: WebRTC: Audio/Video, defect)

Product:
Core
Component:
WebRTC: Audio/Video
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla36
Tracking Flags:
Tracking Status
firefox35 + fixed
firefox36 --- fixed

People

(Reporter: jib, Assigned: ayang)

References

(
URL
)

Details

(Whiteboard: [webrtc-uplift])

Attachments

(1 file)

remove_redundant_capability
1.08 KB, patch
jesup
: review+
lsblakk
: approval-mozilla-beta+
Details | Diff | Splinter Review 
STR 1 (on OSX):
===============
1. Open URL
2. Hit [Capture!] button

Expected:
- You should get 640x480

Actual:
- You get 352x288


STR 2 (on OSX):
===============
1. Open URL
2. Enter values:          Min   Max
             Horizontal  1200, 1920
             Vertical     700, 1920
 3. Hit [Capture!] button

Expected:
- You should get 1280x720

Actual:
- You get 352x288

Regression range:
Last good revision: f547cf19d104
First bad revision: c033be95eb1f
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f547cf19d104&tochange=c033be95eb1f

I.e. it was broken by the MediaEngine video sources refactor (Bug 1053130).

I suspect this is broken on all platforms but I haven't confirmed this.

Unfortunately, we don't have any automated tests yet to keep this from breaking. I'll see if I can fashion at least an OSX-only test out of the URL here.
Err... a redundant member variable. Sorry.
Assignee: nobody → ayang
Attachment #8517888 - Flags: review?(rjesup)
Attachment #8517888 - Flags: review?(rjesup) → review+
One line changed. It is good on my Mac and Linux desktop.
Since no testcase for gUM constraints so far, it should be safe without try.
Keywords: checkin-needed
status-firefox35: --- → affected
status-firefox36: --- → affected
Whiteboard: [webrtc-uplift]
https://hg.mozilla.org/mozilla-central/rev/970f11f7f177
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Can I pick up this bug for testing, if so can someone update test-steps for the same?

PS: Assuming this bug is fixed and ready for QA
Is this safe to uplift to Beta?  If yes, please nominate by Mon Dec 22 for next week's beta.
tracking-firefox35: --- → +
Flags: needinfo?(rjesup)
Comment on attachment 8517888 [details] [diff] [review]
remove_redundant_capability

Approval Request Comment
[Feature/regressing bug #]: bug 1053130

[User impact if declined]: bad aspect ratios on captured video on Mac

[Describe test coverage new/current, TBPL]: on m-c and aurora for quite some time.  Not testable in automation.

[Risks and why]: ultra-low-risk.  Remove a single line that was overriding a variable in a base class (wasn't removed when the class was split into two)

[String/UUID change made/needed]: none


Thanks for the poke, Lukas!
Flags: needinfo?(rjesup)
Attachment #8517888 - Flags: approval-mozilla-beta?
Attachment #8517888 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reproduced the original issue (both scenarios from comment 0) on Mac OS X 10.9.5 using 36 Nightly from November 5. The issue no longer reproduces on Firefox 35 Beta 8 (BuildID=20141229214612) and latest 36 Aurora (BuildID=20150105004007). Note that for the second scenario I get 1200x700, which is what I was also getting for older versions like 34 & 33.

A quick check on Win 7 x64 and Ubuntu 14.04 x64 also gives the expected result for the first scenario. For the second scenario either I cannot share the camera (for the latest Aurora 36) or I get a different resolution (640x480 or 160x120 - for Firefox 35 Beta 8)... the Beta behavior is consistent with behavior for Firefox 33.1, so should not be related to this issue. Any idea why I'm not prompted to share the camera on Aurora for Windows and Linux for the second scenario?
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #11)
> Note that for the second scenario I get 1200x700,

What hardware was this on? I'm also on OS X 10.9.5 - a late-2013 retina MBP - and I get 1280x720. I'm curious because we basically hard-code these defaults on OSX since this OS tells us squat about cameras.

> A quick check on Win 7 x64 and Ubuntu 14.04 x64 also gives the expected
> result for the first scenario. For the second scenario either I cannot share
> the camera (for the latest Aurora 36) or I get a different resolution
> (640x480 or 160x120 - for Firefox 35 Beta 8)... the Beta behavior is
> consistent with behavior for Firefox 33.1, so should not be related to this
> issue. Any idea why I'm not prompted to share the camera on Aurora for
> Windows and Linux for the second scenario?

36 is different because Bug 1003274 was fixed then. Failing outside of the set constraints is the right behavior because the example URL uses required constraints.

For the second scenario, try smaller values. All we need to see is the constraints having an effect on the outcome. On Windows and Ubuntu we get hardware camera abilities, so what limits work will depend on what camera you have.
(In reply to Jan-Ivar Bruaroey [:jib] from comment #12)
> What hardware was this on? I'm also on OS X 10.9.5 - a late-2013 retina MBP
> - and I get 1280x720. I'm curious because we basically hard-code these
> defaults on OSX since this OS tells us squat about cameras.

This is on a late 2013 MacBook Pro 15 inch Retina. When trying different values I see that Firefox just uses those exact values set for the Min. So if the minimum Horizontal and Vertical is 1200x700 that's what's used... same for 800x600 or 1024x768.

> For the second scenario, try smaller values. All we need to see is the
> constraints having an effect on the outcome. On Windows and Ubuntu we get
> hardware camera abilities, so what limits work will depend on what camera
> you have.

For Windows and Ubuntu I did some additional testing with other values, but the results are a bit weird (especially for Ubuntu):

1. Windows (Max=1920 - Horizontal & Vertical)
- Min=1200x700 => 640x480
- Min=1024x768 => 640x480
- Min=800x600 => 640x480
- Min=640x480 or smaller => 640x480
It seems like 640x480 is the max for the camera so this is what's used for any Min values set for the constraints.

2. Ubuntu (Max=1920 - Horizontal & Vertical)
- Min=1200x700 => 160x120
- Min=800x600 => 160x120
- Min=640x480 or smaller => 640x480
It would seem like for Min values smaller than 640x480 we display at 640x480 (which would seem to be the maximum capability of the camera, so rather expected), but for values larger than 640x480 we display at 160x120, which seems a bit weird to me. It seems that if the camera does not support more than the set Min then we display at a much smaller resolution than the maximum supported by the camera. Does this sound right? It looks like it's the opposite from what happens on Windows, but maybe a separate issue from this.
One thing I did not mention: I use different webcam models for Windows and Ubuntu... not sure if this matters in any way.
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #13)
> This is on a late 2013 MacBook Pro 15 inch Retina. When trying different
> values I see that Firefox just uses those exact values set for the Min. So
> if the minimum Horizontal and Vertical is 1200x700 that's what's used...
> same for 800x600 or 1024x768.

Odd, that sounds like the same model I have. Is the image vertically squished a little bit?

> For Windows and Ubuntu I did some additional testing with other values, but
> the results are a bit weird (especially for Ubuntu):
> 
> 1. Windows (Max=1920 - Horizontal & Vertical)
> - Min=1200x700 => 640x480
> - Min=1024x768 => 640x480
> - Min=800x600 => 640x480
> - Min=640x480 or smaller => 640x480
> It seems like 640x480 is the max for the camera so this is what's used for
> any Min values set for the constraints.

On 34-35 this is expected, but unfortunately doesn't prove the constraints work. On 36 all but the last one should fail. Please test that and then in 34-36 please test setting Min=0 and try smaller values for Max to explore any lower-resolution modes your Windows camera may have. As long as they affect the outcome I'm happy.

> 2. Ubuntu (Max=1920 - Horizontal & Vertical)
> - Min=1200x700 => 160x120
> - Min=800x600 => 160x120
> - Min=640x480 or smaller => 640x480
> It would seem like for Min values smaller than 640x480 we display at 640x480
> (which would seem to be the maximum capability of the camera, so rather
> expected), but for values larger than 640x480 we display at 160x120, which
> seems a bit weird to me. It seems that if the camera does not support more
> than the set Min then we display at a much smaller resolution than the
> maximum supported by the camera. Does this sound right? It looks like it's
> the opposite from what happens on Windows, but maybe a separate issue from
> this.

Is this with 34-35? Again I expect all but the last one to fail in 36+. Also, please do the same steps I suggest for windows above.

160x120 is hardly ideal, but not unexpected with the old algorithm. I'm working on implementing the new "ideal"-algorithm to spec right now, so it should work better once that's done.
Oh and feel free to file a bug on 160x120, then I can hang my patches off of that one.
(In reply to Jan-Ivar Bruaroey [:jib] from comment #15)
> Odd, that sounds like the same model I have. Is the image vertically
> squished a little bit?

Yes, it does look a bit squished.

> On 34-35 this is expected, but unfortunately doesn't prove the constraints
> work. On 36 all but the last one should fail. Please test that and then in
> 34-36 please test setting Min=0 and try smaller values for Max to explore
> any lower-resolution modes your Windows camera may have. As long as they
> affect the outcome I'm happy.

1A. Windows + latest Firefox 36 Aurora (Max=1920 - Horizontal & Vertical)
- Min=1200x700 => NO Camera to share
- Min=1024x768 => NO Camera to share
- Min=800x600 => 640x480
- Min=640x480 or smaller => 640x480

It seems the limit for the camera sharing to fail is somewhere between 1024x768 and 800x600.

1B. Windows + Firefox 34.0.5/35 RC/36 latest (Min=0 - Horizontal & Vertical)
- all values tried for Max give 640x480 video (1680x1050, 1280x720, 1200x700, 1024x768, 800x600, 640x480, 320x240, 160x120). With Min=0, 640x480 is all I get.
 
> Is this with 34-35? Again I expect all but the last one to fail in 36+.
> Also, please do the same steps I suggest for windows above.
> 
> 160x120 is hardly ideal, but not unexpected with the old algorithm. I'm
> working on implementing the new "ideal"-algorithm to spec right now, so it
> should work better once that's done.

Yes the results for Ubuntu were taken for 35.

2A. Ubuntu + latest Firefox 36 Aurora (Max=1920 - Horizontal & Vertical)
- Min=1200x700 => NO Camera to share
- Min=1024x768 => NO Camera to share
- Min=800x600 => NO Camera to share
- Min=640x480 or smaller => 640x480

This seems to fail over 640x480 as expected.

2B. Ubuntu + Firefox 34.0.5/35 RC/36 latest (Min=0 - Horizontal & Vertical)
- Max=640x480 or higher => 640x480 
- Max=320x240 => 320x240 
- Max=160x120 => 160x120

(In reply to Jan-Ivar Bruaroey [:jib] from comment #16)
> Oh and feel free to file a bug on 160x120, then I can hang my patches off of
> that one.

I added bug 1119335.
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #17)
> 1A. Windows + latest Firefox 36 Aurora (Max=1920 - Horizontal & Vertical)
> - Min=800x600 => 640x480

I've filed Bug 1119852 on this. Turns out this was a separate regression in FF 32 that nobody caught, since we've got no automated tests for this part of the camera stack. I really have to work on that. :-(
Ugh, I meant to say the regression was in FF 33.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: