Jay Reporter
	Description • 10 years ago

[Blocking Requested - why for this release]:

Steps to reproduce:
1. Launch Loop client app and establish video call between 8x10 devices
2. During video call select Home page
3. Now launch Camera app and observe black camera preview
4. After 5sec. bring back the on-going video call
5. Terminate the call
6. Go back to camera app
7. Observe the preview is still blank

Issue:
Observed preview is blank and control button doesn't work

Expected behaviors:
1. During video call should not allow user to launch camera app
2. If user able to launch camera app, after closing VT session, camera preview resume

	Randell Jesup [:jesup] (needinfo me)
	Comment 1 • 10 years ago

There is only one hardware encoder and one decoder.  Also, I don't think the camera app and webrtc can both access the camera at the same time.  A webrtc call will (if h.264 is used) use the only encoder and decoder until the call ends, and any getUserMedia usage (not just a call) will hold the camera until it's ended.

Perhaps the camera app should recheck the camera if it was unavailable before.

	Diego Wilson [:diego]
	Comment 2 • 10 years ago

(In reply to Randell Jesup [:jesup] from comment #1)
> There is only one hardware encoder and one decoder.  Also, I don't think the
> camera app and webrtc can both access the camera at the same time.  A webrtc
> call will (if h.264 is used) use the only encoder and decoder until the call
> ends, and any getUserMedia usage (not just a call) will hold the camera
> until it's ended.
> 
> Perhaps the camera app should recheck the camera if it was unavailable
> before.

So you believe this is more of a camera app bug?

	Randell Jesup [:jesup] (needinfo me)
	Comment 3 • 10 years ago

(In reply to Diego Wilson [:diego] from comment #2)
> (In reply to Randell Jesup [:jesup] from comment #1)
> > There is only one hardware encoder and one decoder.  Also, I don't think the
> > camera app and webrtc can both access the camera at the same time.  A webrtc
> > call will (if h.264 is used) use the only encoder and decoder until the call
> > ends, and any getUserMedia usage (not just a call) will hold the camera
> > until it's ended.
> > 
> > Perhaps the camera app should recheck the camera if it was unavailable
> > before.
> 
> So you believe this is more of a camera app bug?

Per our conversation on the call - yes.  The camera app should put up an error if it can't get the camera because another application has it.  Likely when the camera app gets re-focused and doesn't have the camera, it should try to re-acquire it, but I don't know the app model for the Camera app that well.  Perhaps it should exit after notifying the user if it tries to get the camera and can't. 

MikeH - do you know who should look into this for the Camera app?

	Justin D'Arcangelo [:justindarc]
	Comment 4 • 10 years ago

It appears as though we are simply giving up after 3 attempts at getting the camera hardware:

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/camera.js#L339

We probably *should* be displaying some sort of overlay saying that the camera couldn't be acquired. However, I would expect that the Loop client should be releasing the camera hardware when it goes to the background.

Randell: Can you confirm that the Loop client is releasing the camera hardware when the app loses focus?

	Randell Jesup [:jesup] (needinfo me)
	Comment 5 • 10 years ago

I can't confirm that (and believe it's not true), but I can forward the request.  I'll note that if the app is still "running", you're still in a call - if the app shuts down the call on losing focus, then the camera should be released.  However, from comment 0, note that the call does not end when it loses focus, and so the camera must remain active.

	Justin D'Arcangelo [:justindarc]
	Comment 6 • 10 years ago

(In reply to Randell Jesup [:jesup] from comment #5)
> I can't confirm that (and believe it's not true), but I can forward the
> request.  I'll note that if the app is still "running", you're still in a
> call - if the app shuts down the call on losing focus, then the camera
> should be released.  However, from comment 0, note that the call does not
> end when it loses focus, and so the camera must remain active.

Even if there's still an active call, the camera hardware *should* be released when the Loop app loses focus. But, I'm not familiar enough with the Loop client to know how feasible that is. It should be just a matter of listening for `visibilitychange` on the `document` object and calling `release()` on the mozCamera object when `document.hidden === true`.

	Randell Jesup [:jesup] (needinfo me)
	Comment 7 • 10 years ago

(In reply to Justin D'Arcangelo [:justindarc] from comment #6)
> Even if there's still an active call, the camera hardware *should* be
> released when the Loop app loses focus. But, I'm not familiar enough with
> the Loop client to know how feasible that is.

Note: this is a video call, and the video call is continuing, so we can't release the camera (unless we end the call).

> It should be just a matter of
> listening for `visibilitychange` on the `document` object and calling
> `release()` on the mozCamera object when `document.hidden === true`.

This is getUserMedia() using the camera at a lower level, we'd have to do stream.stop() to stop capture and release the hardware.

	Daniel Coloma:dcoloma
	Comment 8 • 10 years ago

Loop is not releasing the Camera when moving to the Background because the video call should continue while the user decides the call should still be active. 

We have detected recently some issues, especially with the back camera (in a Flame), where the camera stream cannot be properly acquired, we filed bug 1080657 today and a patch is already available.

	Justin D'Arcangelo [:justindarc]
	Comment 9 • 10 years ago

(In reply to Randell Jesup [:jesup] from comment #7)
> (In reply to Justin D'Arcangelo [:justindarc] from comment #6)
> > Even if there's still an active call, the camera hardware *should* be
> > released when the Loop app loses focus. But, I'm not familiar enough with
> > the Loop client to know how feasible that is.
> 
> Note: this is a video call, and the video call is continuing, so we can't
> release the camera (unless we end the call).
> 

Is there a privacy concern of continuing to stream the camera when the app is not in the foreground? I would expect the person on the receiving end of the video call to get a "Video Stream Unavailable" or something similar when the caller's app goes to the background.

	KTucker [:KTucker][Inactive 3/4/2016]
	Comment 10 • 10 years ago

Adding qawanted for branch checks.

	Jayme Mercado [:JMercado] (Inactive 3/4/2016)
	Comment 11 • 10 years ago

This issue occurs on 2.2 Flame KK, 2.1 Flame KK, and 2.0 Flame KK.

Acutal Results: The loop preview (and what is sent to the connected device) freezes after opening the camera app.

Environmental Variables (shallow flash):
Device: Flame 2.2
BuildID: 20141012115053
Gaia: 3b81896f04a02697e615fa5390086bd5ecfed84f
Gecko: 199fb29c3467
Version: 35.0a1 (2.2) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Environmental Variables (shallow flash):
Device: Flame 2.1
BuildID: 20141011150624
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: e96a7a4f3bbe
Version: 34.0a2 (2.1) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables (shallow flash):
Device: Flame 2.0
BuildID: 20141010074705
Gaia: 6effca669c5baaf6cd7a63c91b71a02c6bd953b3
Gecko: 54ec9cb26b59
Version: 32.0 (2.0) 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Loop Version for all builds: 59294fd

	Randell Jesup [:jesup] (needinfo me)
	Comment 12 • 10 years ago

(In reply to Justin D'Arcangelo [:justindarc] from comment #9)
> (In reply to Randell Jesup [:jesup] from comment #7)
> > (In reply to Justin D'Arcangelo [:justindarc] from comment #6)
> > > Even if there's still an active call, the camera hardware *should* be
> > > released when the Loop app loses focus. But, I'm not familiar enough with
> > > the Loop client to know how feasible that is.
> > 
> > Note: this is a video call, and the video call is continuing, so we can't
> > release the camera (unless we end the call).
> > 
> 
> Is there a privacy concern of continuing to stream the camera when the app
> is not in the foreground? I would expect the person on the receiving end of
> the video call to get a "Video Stream Unavailable" or something similar when
> the caller's app goes to the background.

It's an issue, but on the other hand people get annoyed if a call drops (or goes dead) because the person flipped to Yelp or Maps or Contacts to get info they want to tell the person they're talking to.

The same issue comes up with desktop, and we try to ensure people know the camera/mic are active.  (if you're in a 3G phone call, and you press the home button to look something up, does the call end?  (I don't have a SIM so I haven't tried that).

If we were to end it on interacting with something else (does that include pulling down the statusbar?), then we'd need to have some way to halt your action to allow a "this will end your call", "ok/cancel".

All the phones I'm used to allow calls to continue int he background, and try to make this obvious in the UI.

	Andrew Overholt [:overholt]
	Comment 13 • 10 years ago

CAF 2.1 FC blocker + what appears to be active ongoing discussion about what to do here = 2.1+ in the opinion of those doing triage today.

	Justin D'Arcangelo [:justindarc]
	Comment 14 • 10 years ago

Randell: Flagging you for feedback. This is a patch for Camera that puts up a full-screen overlay when the camera hardware cannot be acquired by the app. This should more gracefully handle the case when a user opens the Camera app in the middle of a WebRTC video call.

Diego: Please review. If you don't have time, just re-assign it. Thanks!

	Justin D'Arcangelo [:justindarc]
	Comment 15 • 10 years ago

Randell, I just tested out a video call with Hema using FaceTime and while on the video call, she left the FaceTime app to return to the homescreen. When she did that, the video stream on my end *stopped* and left the audio stream open so we were still able to communicate. I believe that this is what the expected behavior should be for a WebRTC call. This would relinquish control over the camera hardware and alleviate the privacy concerns without abruptly interrupting the call. I've attached a screenshot of the "paused" FaceTime call.

Also note, I've attached a PR to this bug that patches the Camera app to show a full-screen overlay informing the user that the camera is already in use which should also help resolve this issue. Regardless of whether or not we implement the ability for WebRTC to stop the video stream and leave the audio stream open, the Camera app needs this patch to prevent potential 3rd-party camera apps from breaking the Gaia Camera app.

	Justin D'Arcangelo [:justindarc]
	Comment 16 • 10 years ago

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/f0b9b9098d731b55faeba8fd2da268b9ecce4d49

	Justin D'Arcangelo [:justindarc]
	Comment 17 • 10 years ago

Comment on attachment 8506340 [details]
pull-request (master)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): none
[User impact] if declined: User is presented with an unusable UI in Camera when on a WebRTC call
[Testing completed]: Tested on Flame, added additional unit tests
[Risk to taking this patch] (and alternatives if risky): l10n risk (new strings added)
[String changes made]:

- Added string: camera-unavailable-title
- Added string: camera-unavailable-text

	bhavana bajaj [:bajaj]
	Comment 18 • 10 years ago

(In reply to Justin D'Arcangelo [:justindarc] from comment #17)
> Comment on attachment 8506340 [details]
> pull-request (master)
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): none
> [User impact] if declined: User is presented with an unusable UI in Camera
> when on a WebRTC call
> [Testing completed]: Tested on Flame, added additional unit tests
> [Risk to taking this patch] (and alternatives if risky): l10n risk (new
> strings added)
> [String changes made]:
> 
> - Added string: camera-unavailable-title
> - Added string: camera-unavailable-text

Justin, we are way past the string freeze deadline for 2.1 so this solution cannot be uplifted there. If this needs to be resolved on 2.1, we would need alternative solution. Please keep us posted on any alternatives. I will also sync-up with CAF offline to see if they have any input here.

	Justin D'Arcangelo [:justindarc]
	Comment 19 • 10 years ago

(In reply to bhavana bajaj [:bajaj] from comment #18)
> (In reply to Justin D'Arcangelo [:justindarc] from comment #17)
> > Comment on attachment 8506340 [details]
> > pull-request (master)
> > 
> > [Approval Request Comment]
> > [Bug caused by] (feature/regressing bug #): none
> > [User impact] if declined: User is presented with an unusable UI in Camera
> > when on a WebRTC call
> > [Testing completed]: Tested on Flame, added additional unit tests
> > [Risk to taking this patch] (and alternatives if risky): l10n risk (new
> > strings added)
> > [String changes made]:
> > 
> > - Added string: camera-unavailable-title
> > - Added string: camera-unavailable-text
> 
> Justin, we are way past the string freeze deadline for 2.1 so this solution
> cannot be uplifted there. If this needs to be resolved on 2.1, we would need
> alternative solution. Please keep us posted on any alternatives. I will also
> sync-up with CAF offline to see if they have any input here.

Bhavana: That's what I thought. The only other solution would be for the Loop/WebRTC client to release the camera hardware when it is no longer in the foreground as I have mentioned in the previous comments. However, it seems that the Loop/WebRTC client is unable to do that (see Comment 12).

However, I have pinged Randell for NI? in Comment 15 to see if it would be possible for the Loop/WebRTC client to adopt a similar UX as FaceTime (e.g. release the camera when the app enters the background and the person on the receiving end of the call would then be limited to an audio-only stream with a "Paused" or "No Video Available" message on-screen).

	Justin D'Arcangelo [:justindarc]
	Comment 20 • 10 years ago

Re-opening this to track the issue specific to WebRTC/Loop. The camera error overlay patch has already landed and has been moved over to Bug 1080677.

	Hema Koka [:hema]
	Comment 21 • 10 years ago

> 
> However, I have pinged Randell for NI? in Comment 15 to see if it would be
> possible for the Loop/WebRTC client to adopt a similar UX as FaceTime (e.g.
> release the camera when the app enters the background and the person on the
> receiving end of the call would then be limited to an audio-only stream with
> a "Paused" or "No Video Available" message on-screen).

Randall, 

Can we just release the camera when webrtc call goes to the background and re-acquire when you bring the loop app back into foreground. It looks like webrtc call ending is tightly coupled with camera streaming from previous comments. As Justin already mentioned we were trying to compare with other applications such as facetime and when a camera-based app goes to background, video stream is not shown for video calls. 

On the camera app side, we release the stream when the app goes to the background and re-acquire it (3 attempts 1000ms apart) when it comes back up in the foreground. If the hardware is not acquired, we should be displaying an error message on the camera app. Justin has already fixed this on master: https://bugzilla.mozilla.org/show_bug.cgi?id=1080677. As it has string impact we cannot get that patch into 2.1 as this stage. 

Please check to see if we can get to a feasible solution/workaround given where we are in the 2.1 release cycle. 

Thanks
Hema

	Randell Jesup [:jesup] (needinfo me)
	Comment 22 • 10 years ago

I'll look.  Currently I suspect it's not even close to feasible to do in the 2.1 timeframe (even just for b2g only), as it was never designed to release the camera and re-acquire it; it would be a significant rework as the interfaces are asynchronous.  Also, some webrtc applications may take loss of video to mean call failure, so we may need to pump black frames in off a timer (which we don't have currently) to keep the call alive.  Perhaps there's a shortcut/hack, though.

	bhavana bajaj [:bajaj]
	Comment 23 • 10 years ago

:jesup/Justin, we are waiting to see if you guys have other thoughts/or hacks here and in the interim I am communicating with CAF on seeing if we can fix this is 2.2 instead given the limitation on strings.

	Jay Reporter
	Comment 24 • 10 years ago

Randell, Justin
Is it possible at least just exit the camera app to avoid the screen freezes if it's too late to pull in the fixe?

	Hema Koka [:hema]
	Comment 25 • 10 years ago

Stephany is helping check with L10n guys if we can take this: https://bugzilla.mozilla.org/show_bug.cgi?id=1080677

Leaving an NI for both Stephany and Justin

	Stephany Wilkes
	Comment 26 • 10 years ago

Juwei is the UX person on Loop so I'm adding her to this thread. She should have been flagged on ui-review? earlier for proposed solutions.

It sounds like the current proposal is to show the user a message when the user is on a WebRTC call, attempts to access the camera, and cannot because they are currently in a WebRTC call. But, we are also in the late-l10n timeframe. 

Stas, can you let me know if we have any existing error strings for camera, already translated, that we might be able to use here?

	Justin D'Arcangelo [:justindarc]
	Comment 27 • 10 years ago

Stephany, here are the l10n strings for Camera in v2.1 (nothing applicable to this condition):

https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/camera/locales/camera.en-US.properties

	Justin D'Arcangelo [:justindarc]
	Comment 28 • 10 years ago

(In reply to Jay from comment #24)
> Randell, Justin
> Is it possible at least just exit the camera app to avoid the screen freezes
> if it's too late to pull in the fixe?

Is this acceptable behavior? It seems to me that this would be just as bad as what happens currently (blank viewfinder). Perhaps we can get UX input on this?

	Hema Koka [:hema]
	Comment 29 • 10 years ago

(In reply to Stephany Wilkes from comment #26)
> Juwei is the UX person on Loop so I'm adding her to this thread. She should
> have been flagged on ui-review? earlier for proposed solutions.
> 
> It sounds like the current proposal is to show the user a message when the
> user is on a WebRTC call, attempts to access the camera, and cannot because
> they are currently in a WebRTC call. But, we are also in the late-l10n
> timeframe. 
> 
> Stas, can you let me know if we have any existing error strings for camera,
> already translated, that we might be able to use here?

Steph, We couldn't find anything close from the camera strings list. Perhaps a generic message like "Service Unavailable" or "Not Available" could possibly be borrowed from other apps or from shared. We need to make sure the translations for this in various languages are also generic like the english strings.

Thanks
Hema

	Juwei Huang[:juwei]
	Comment 30 • 10 years ago

Ni? Mike instead since the UX person will be re-assign.

	Randell Jesup [:jesup] (needinfo me)
	Comment 31 • 10 years ago

Just confirming (Sorry, I thought I had already): there's no safe way to modify getUserMedia (note: not really WebRTC) to allow for the camera to be released/stolen when we're not in foreground for 2.1, and then get it back when we come forward.  (It's also unclear what the "right" action is here; some usecases would keep it live while you look up info, others would relegate the call video to a small overlay, others would mute the video as has been discussed here -- but those are questions for 2.2 or later.)

	Hema Koka [:hema]
	Comment 32 • 10 years ago

We not quite sure if we can borrow strings from else where (outside of camera) to use as a workaround. This issue is pretty bad and needs to be fixed at least with a generic message workaround. 

Adding Jeff Beatty. Stephany mentioned that you may have ideas to help here.

Worst case: We really need to get the new string translated that Justin already added (its on master). Appreciate help with localization.

	Stephany Wilkes
	Comment 33 • 10 years ago

In order to be even somewhat acceptable, especially in a big new feature and *CAF blocker* like this, one of the following must happen. Either:

* We make an exception for late-l10N and translate and uplift the new string from Justin on master; 

* We figure out how to use an existing string to avoid late-l10n. This is a technical issue that folks aren't sure how to do, given the way strings are used in the OS (not easy to work with);

* We push this to 2.3. 

I will raise this to Chris Lee, Axel and Bhavana to help figure out a way forward here.

	Axel Hecht [:Pike]
	Comment 34 • 10 years ago

There's no 2.2 l10n to pick up from, we're just about to start on that due to infrastructure blockers.

Clearing needinfo on me and jbeatty, I don't think either of us have a lot to add.

	Chris Lee [:clee]
	Comment 35 • 10 years ago

> * We make an exception for late-l10N and translate and uplift the new string
> from Justin on master; 

Let's explore using an existing string first to see if it's acceptable and makes sense to the end user.  Then if necessary, understand the impact on taking this late-l10n string change.  

ni? to Delphine for her initial assessment on this for l10n.  

Once we get updates on the items above, we can make a decision on how to proceed.  Thanks.

	Hema Koka [:hema]
	Comment 36 • 10 years ago

Adding DJF since he has another idea of a gecko fix

	David Flanagan [:djf]
	Comment 37 • 10 years ago

Actually my idea for a gecko fix was specific to the <video> element issue where other videos can't play. I'd forgotten that for this bug it is the camera hardware that is in use, in addition to the video decoding hardware.

The things I'd add are:

1) If we can come up with some sort of suitable graphic that conveys "camera in use" then we can display that instaed of text and avoid the l10n issue.

2) Hema's suggestion about borrowing strings from other apps is intriguing. If it involved loading the entire locale file for the other app, that would have performance issues and could break existing strings if there were naming conflicts.  But if Axel has a way to just copy a localized string from one app to another, then we should look at other apps for better strings. Axel: is there any way to do that?

	Justin D'Arcangelo [:justindarc]
	Comment 38 • 10 years ago

(In reply to David Flanagan [:djf] from comment #37)
> Actually my idea for a gecko fix was specific to the <video> element issue
> where other videos can't play. I'd forgotten that for this bug it is the
> camera hardware that is in use, in addition to the video decoding hardware.
> 
> The things I'd add are:
> 
> 1) If we can come up with some sort of suitable graphic that conveys "camera
> in use" then we can display that instaed of text and avoid the l10n issue.
> 

Great suggestion!

Stephany: Can we get a graphic/icon from UX made that looks like a camera with an "X" over it or something along those lines?

	Axel Hecht [:Pike]
	Comment 39 • 10 years ago

I don't have a way to cherry pick strings from one app into another. That'd likely be build-foo, in the email thread I've punted that question to gandalf. Nothing that I expect to be straight-forward though, and probably something that involves risk.

	chris hofmann
	Comment 40 • 10 years ago

delphine is out until monday.

seems like every hardware compoent of the firefoxOS device needs a "not avaliable" and/or device contention issues messages


like we already have for:
	radio not available
	memory card not available
	sim not available

http://transvision.mozfr.org/?recherche=not+available&repo=gaia&sourcelocale=en-US&locale=en-US&search_type=strings

I bet if we code reviewed/tested extensively we would find that we already have problems like this beyond just contention for the camera.

Given these things I'd suggest just try and do the right thing for the user without presenting UI by shutting down or not responding to conflicting use of the camera/other component on the device.

Second best choice is just to show hard coded/English strings and convince ourselves this is a side case not needing attention in the first release.

trying to swipe an existing "Not Available" string from another app for all locales at build time (or other) seems prone to error and fragile.

	Justin D'Arcangelo [:justindarc]
	Comment 41 • 10 years ago

What if we just show an indefinite spinner? Thoughts?

	Stephany Wilkes
	Comment 42 • 10 years ago

I'd accept hard-coded English strings, given the tight timeline. It can be a Plan B to what I now understand are a few engineers looking at a bug fix for all of these cases.

	Justin D'Arcangelo [:justindarc]
	Comment 43 • 10 years ago

Here's a screenshot of the full-screen overlay shown to the user when the camera hardware is unavailable (on a Loop call). This is on master and landed in Bug 1080677.

	Justin D'Arcangelo [:justindarc]
	Comment 44 • 10 years ago

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): none
[User impact] if declined: User is presented with an unusable UI in Camera when on a WebRTC call
[Testing completed]: Tested on Flame, added additional unit tests
[Risk to taking this patch] (and alternatives if risky): l10n risk (new strings added)
[String changes made]:

- Added string: camera-unavailable-title
- Added string: camera-unavailable-text

Hema: Here's the uplifted v2.1 version of the patch that adds the error message overlay to Camera. The two added l10n strings are noted above.

	Stephany Wilkes
	Comment 45 • 10 years ago

Flagging Eric (who is in EU time zone and would be able to take this sooner than Amy) to see if he can provide an icon that communicates "camera not available," something like a camera under the slash/bullseye or similar. Flagging Amy so she's aware of this request.

The icon is something that could be shown besides or in addition to a string for users who don't speak English and may not understand the hard-coded string.

	Hema Koka [:hema]
	Comment 46 • 10 years ago

Based on comment 42, To avoid the bad user experience, we are going with new clear message workaround. We plan to leverage the 2.2 translations as they are made available and cherry pick them into 2.1 branch (follow-up bug for cherry picking the translations: https://bugzilla.mozilla.org/show_bug.cgi?id=1091303). 

In addition as Stephany mentioned in previous comment we are also exploring adding an image that indicates the "camera is in use".

If there are better options, feel free to suggest.

Thanks
Hema

	bhavana bajaj [:bajaj]
	Comment 47 • 10 years ago

Comment on attachment 8513895 [details]
pull-request (v2.1)

Approving the exception with string changes given the discussion on the bug and offline(over the email) agreement here and given the timeline to get this fixed.

	Eric Pang [:epang] UX
	Comment 48 • 10 years ago

I've an icons for 'camera unavailable' in the various sizes.

	Eric Pang [:epang] UX
	Comment 49 • 10 years ago

(In reply to Eric Pang [:epang] from comment #48)
> Created attachment 8514442 [details]
> Camera Unavailable Icons
> 
> I've an icons for 'camera unavailable' in the various sizes.

attached :)

	Hema Koka [:hema]
	Comment 50 • 10 years ago

(In reply to Eric Pang [:epang] from comment #49)
> (In reply to Eric Pang [:epang] from comment #48)
> > Created attachment 8514442 [details]
> > Camera Unavailable Icons
> > 
> > I've an icons for 'camera unavailable' in the various sizes.
> 
> attached :)

Thanks Eric for a quick turnaround on this!

	Justin D'Arcangelo [:justindarc]
	Comment 51 • 10 years ago

Eric: Thanks for the quick turnaround on the icon. Please provide ui-review for the updated overlay. I've attached a screenshot for your convenience. Thanks!

	Eric Pang [:epang] UX
	Comment 52 • 10 years ago

Comment on attachment 8514504 [details]
"Camera Unavailable" screenshot WITH ICON

Thanks Justin!  Can we change it so the icons is on top and the text is below?  This will be more consistent with the the FM Radio screen.  I'll post a screen so you can see the positioning, thanks!  Flag me for review when ready and feel free to ping me on irc for a faster review :)

	Eric Pang [:epang] UX
	Comment 53 • 10 years ago

Hey Justin, here's the FM radio screen.  You can ignore the textured background, just wanted to show you the positioning of the icons and text.  If we can have the icon in a similar locations with the text vertically centered below we should be good.  Thanks and let me know if you have any questions!

	Justin D'Arcangelo [:justindarc]
	Comment 54 • 10 years ago

Eric: I've updated the overlay. Let me know how this looks. Thanks!

	Justin D'Arcangelo [:justindarc]
	Comment 55 • 10 years ago

Updated the overlay padding to match the FM overlay a little better.

	Justin D'Arcangelo [:justindarc]
	Comment 56 • 10 years ago

Let's try this again. More tweaks to spacing.

	Amy Lee [:amylee] UX
	Comment 57 • 10 years ago

(In reply to Stephany Wilkes from comment #45)
> Flagging Eric (who is in EU time zone and would be able to take this sooner
> than Amy) to see if he can provide an icon that communicates "camera not
> available," something like a camera under the slash/bullseye or similar.
> Flagging Amy so she's aware of this request.
> 
> The icon is something that could be shown besides or in addition to a string
> for users who don't speak English and may not understand the hard-coded
> string.

	Eric Pang [:epang] UX
	Comment 58 • 10 years ago

Comment on attachment 8515162 [details]
"Camera Unavailable" screenshot WITH ICON

Looks good to me, thanks Justin! R+

	Justin D'Arcangelo [:justindarc]
	Comment 59 • 10 years ago

Landed on v2.1:

https://github.com/mozilla-b2g/gaia/commit/903f6805ec3959c6bdb84d44100981dbac3903cb

	Delphine Lebédel [:delphine - Off until Sept 19]
	Comment 60 • 10 years ago

Clearing the needinfo as I was on PTO and this was resolved since then

	[:fabrice] Fabrice Desré
	Comment 61 • 10 years ago

Comment on attachment 8506340 [details]
pull-request (master)

Not sure why you ask for approval to land on master. You don't need it!

	Yeojin Chung [:YeojinC]
	Comment 62 • 10 years ago

This issue STILL reproduces on Flame 2.1 and 2.2.

Result: I was able to reproduce the exact same result from Comment 0. Camera app displays a blank preview screen. Also, when the user opens the Camera app during a webRTC call, no error/warning message is presented.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141107001205
Gaia: 6295f6acfe91c6ae659712747dd2b9c8f51d0339
Gecko: 8c23b4f2ba29
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 (319mb, KK, Full Flash)
BuildID: 20141107040206
Gaia: 779f05fead3d009f6e7fe713ad0fea16b6f2fb31
Gecko: 64f4392d0bdc
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

	Justin D'Arcangelo [:justindarc]
	Comment 63 • 10 years ago

(In reply to Yeojin Chung [:YeojinC] from comment #62)
> This issue STILL reproduces on Flame 2.1 and 2.2.
> 
> Result: I was able to reproduce the exact same result from Comment 0. Camera
> app displays a blank preview screen. Also, when the user opens the Camera
> app during a webRTC call, no error/warning message is presented.
> 
> Device: Flame 2.1 (319mb, KK, Shallow Flash)
> BuildID: 20141107001205
> Gaia: 6295f6acfe91c6ae659712747dd2b9c8f51d0339
> Gecko: 8c23b4f2ba29
> Version: 34.0 (2.1) 
> Firmware Version: v188-1
> User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
> 
> Device: Flame 2.2 (319mb, KK, Full Flash)
> BuildID: 20141107040206
> Gaia: 779f05fead3d009f6e7fe713ad0fea16b6f2fb31
> Gecko: 64f4392d0bdc
> Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
> Version: 36.0a1 (2.2 Master)
> Firmware: V188
> User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

That's strange. I'm seeing the same thing too.

Mike/Andrew: Has something changed in the last few days to MozCamera that would cause the `onError()` callback for `mozCameras.getCamera()` to never fire if the hardware is busy?

Here is the call to `getCamera`:

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/camera.js#L294

Here is the `onError` handler that never gets called:

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/camera.js#L346

	Justin D'Arcangelo [:justindarc]
	Comment 64 • 10 years ago

(In reply to Justin D'Arcangelo [:justindarc] from comment #63)
> Mike/Andrew: Has something changed in the last few days to MozCamera that
> would cause the `onError()` callback for `mozCameras.getCamera()` to never
> fire if the hardware is busy?
> 
> Here is the call to `getCamera`:
> 
> https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/
> camera.js#L294
> 
> Here is the `onError` handler that never gets called:
> 
> https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/
> camera.js#L346

Could this have anything to do with Bug 1090501 ?

	Mike Habicher [:mikeh] (high bugzilla latency) Assignee
	Comment 65 • 10 years ago

(In reply to Justin D'Arcangelo [:justindarc] from comment #64)

> Could this have anything to do with Bug 1090501 ?

It's very possible. I'll take a look.

	Mike Habicher [:mikeh] (high bugzilla latency) Assignee
	Comment 66 • 10 years ago

justin, with this patch I get a "Hardware is in use..." error when the camera hardware can't be opened. Can you give it a try to confirm it works for you as well?

	Andrew Osmond [:aosmond] (he/him)
	Comment 67 • 10 years ago

Comment on attachment 8520879 [details] [diff] [review]
Handle pre-init camera's failure to start

Review of attachment 8520879 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

	Justin D'Arcangelo [:justindarc]
	Comment 68 • 10 years ago

This is a really simple app I've been using for testing when getUserMedia() has a lock on the camera hardware. It simply uses getUserMedia() to load the camera video stream into a <video> element. This should make it easier to test this without making a call via the Loop client.

	Justin D'Arcangelo [:justindarc]
	Comment 69 • 10 years ago

Comment on attachment 8520879 [details] [diff] [review]
Handle pre-init camera's failure to start

Review of attachment 8520879 [details] [diff] [review]:
-----------------------------------------------------------------

This appears to solve the issue on master.

To clarify, I have just now re-tested to see if Camera shows an "unavailable" overlay on both v2.1 and master (v2.2) and the overlay appears to be working properly for me on v2.1. The overlay should also now work properly on master with Mike's patch applied.

v2.1: WORKING
Gaia-Rev        4c159e75a1568afbbf0c83c1235ec56facfbe87d
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/b9849b3c6aaa
Build-ID        20141112001201
Version         34.0

v2.2: BROKEN
Gaia-Rev        5ae28ff11b982e2bd7d1aa097cda131536952bdc
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/688f821edcd4
Build-ID        20141112040208
Version         36.0a1

v2.2 + Patch: WORKING

	Justin D'Arcangelo [:justindarc]
	Comment 70 • 10 years ago

Can we please re-verify based on my findings in Comment 69?

	Mike Habicher [:mikeh] (high bugzilla latency) Assignee
	Comment 71 • 10 years ago

ychung, ktucker -- I'll be landing this patch on b2g-inbound shortly, if that helps your workflow any.

	Mike Habicher [:mikeh] (high bugzilla latency) Assignee
	Comment 72 • 10 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/ab6a050eccb9

	Yeojin Chung [:YeojinC]
	Comment 73 • 10 years ago

I was unable to see the "unavailable" overlay on Flame 2.1. I got the exact same result as Comment 62. I used the Hello app again and followed exact same STRs.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141112001201
Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
Gecko: b9849b3c6aaa
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

	Yeojin Chung [:YeojinC]
	Comment 74 • 10 years ago

(In reply to Yeojin Chung [:YeojinC] from comment #73)
> I was unable to see the "unavailable" overlay on Flame 2.1. I got the exact
> same result as Comment 62. I used the Hello app again and followed exact
> same STRs.
> 
> Device: Flame 2.1 (319mb, KK, Shallow Flash)
> BuildID: 20141112001201
> Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
> Gecko: b9849b3c6aaa
> Version: 34.0 (2.1) 
> Firmware Version: v188-1
> User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Correction: Please disregard Comment 73. I will re-verify this issue once the patch is uplifted on 2.1 and 2.2. Sorry about the confusion.

	Justin D'Arcangelo [:justindarc]
	Comment 75 • 10 years ago

(In reply to Yeojin Chung [:YeojinC] from comment #74)
> (In reply to Yeojin Chung [:YeojinC] from comment #73)
> > I was unable to see the "unavailable" overlay on Flame 2.1. I got the exact
> > same result as Comment 62. I used the Hello app again and followed exact
> > same STRs.
> > 
> > Device: Flame 2.1 (319mb, KK, Shallow Flash)
> > BuildID: 20141112001201
> > Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
> > Gecko: b9849b3c6aaa
> > Version: 34.0 (2.1) 
> > Firmware Version: v188-1
> > User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
> 
> Correction: Please disregard Comment 73. I will re-verify this issue once
> the patch is uplifted on 2.1 and 2.2. Sorry about the confusion.

Yeojin: The patch Mike is landing is only for v2.2. I am unable to reproduce the absence of the "unavailable" overlay on v2.1. Can you please verify if you get the "unavailable" overlay on v2.1 using my sample getUserMedia app?

STR:
1. Install sample getUserMedia app -- https://bugzilla.mozilla.org/attachment.cgi?id=8521527
2. Launch sample getUserMedia app
3. Tap "Home" button to return to homescreen while getUserMedia app is running
4. Launch Camera
5. Observe "Unavailable" overlay

	Yeojin Chung [:YeojinC]
	Comment 76 • 10 years ago

Hi Justin, thanks for the STR. I was able to see the "Camera Unavailable" overlay. 

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141112001201
Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
Gecko: b9849b3c6aaa
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

	Justin D'Arcangelo [:justindarc]
	Comment 77 • 10 years ago

(In reply to Yeojin Chung [:YeojinC] from comment #76)
> Hi Justin, thanks for the STR. I was able to see the "Camera Unavailable"
> overlay. 
> 
> Device: Flame 2.1 (319mb, KK, Shallow Flash)
> BuildID: 20141112001201
> Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
> Gecko: b9849b3c6aaa
> Version: 34.0 (2.1) 
> Firmware Version: v188-1
> User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Yeojin, in that case, you should also see it if the Loop client is using the camera and you attempt to open the Camera app. The example app I provided uses getUserMedia which is the same thing the Loop client is using to access the hardware. If you're still not able to see the "unavailable" message when the Loop client is active, then there may be another issue at large here.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 78 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/ab6a050eccb9

	Yeojin Chung [:YeojinC]
	Comment 79 • 10 years ago

(In reply to Justin D'Arcangelo [:justindarc] from comment #77)

I think the problem is related to the difference between front and back camera. I re-tested with the test app using both cameras. Here's the result:

1. Open getUserMedia app > Select Back Camera > Open Camera app > Overlay appears.
2. Open getUserMedia app > Select Front Camera > Open Camera app > Blank camera preview appears (no overlay).

Video: http://youtu.be/tWu1Rm0Iybs

Since the Loop app uses the front camera, I was unable to get the overlay when I opened the Camera app, which has the back camera as default. Both 2.1 and 2.2 display the same behaviors.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141113001200
Gaia: 569a299ca446f714cd98d5881cc058fd6f6e257b
Gecko: d188e92aa5a6
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 (319mb, KK, Shallow Flash)
BuildID: 20141113040205
Gaia: be8b0151d2f9a4c41fc63952128e0b723cd1161d
Gecko: ab137ddd3746
Version: 36.0a1 (2.2) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

	Mike Habicher [:mikeh] (high bugzilla latency) Assignee
	Comment 80 • 10 years ago

Following the STR in comment 79, I observed the observed behaviour. In the logcat, I see:

11-14 15:47:11.190   228 15348 E mm-camera: mct_bus_sof_thread_run: SOF freeze; Sending error message
11-14 15:47:11.190   203 15333 E mm-camera-intf: mm_camera_event_notify: Camera Event DAEMON DIED received
11-14 15:47:11.190   203 15369 E mm-camera-intf: mm_camera_event_notify: Camera Event DAEMON DIED received
11-14 15:47:11.190 15232 15237 I PRLog   : 31326264[b059a200]: OnSystemError : aWhere=0, aArg1=100, aArg2=0
11-14 15:47:11.190 15232 15237 I PRLog   : 31326264[b059a200]: New preview state is 'stopped'
11-14 15:47:11.190 15232 15237 I PRLog   : 31326264[b059a200]: CameraControlImpl::OnSystemError : aContext='Camera Service' (0), aError=0x80004005
11-14 15:47:11.190 15263 15381 I PRLog   : 25524688[b1b8f180]: OnSystemError : aWhere=0, aArg1=100, aArg2=0
11-14 15:47:11.190 15263 15381 I PRLog   : 25524688[b1b8f180]: New preview state is 'stopped'
11-14 15:47:11.190 15263 15381 I PRLog   : 25524688[b1b8f180]: Preview stopped, clearing current frame
11-14 15:47:11.190 15263 15381 I PRLog   : 25524688[b1b8f180]: CameraControlImpl::OnSystemError : aContext='Camera Service' (0), aError=0x80004005
11-14 15:47:11.190 15263 15263 I PRLog   : -1224769196[b6b4d080]: DOM OnHardwareStateChange: closed

	Mike Habicher [:mikeh] (high bugzilla latency) Assignee
	Comment 81 • 10 years ago

I also see this in the logcat when opening the Camera app:

11-14 15:16:35.860   255  1915 E mm-camera: isp_reserve_src_port: Error isp_src_port

	Mike Habicher [:mikeh] (high bugzilla latency) Assignee
	Comment 82 • 10 years ago

Some more:

11-14 15:16:35.990   255  1915 E mm-camera-sensor: port_sensor_port_process_event:691 failed
11-14 15:16:35.990   255  1915 E mm-camera-sensor: module_sensor_stream_on:1039 failed
11-14 15:16:35.990   255  1915 E mm-camera-sensor: module_sensor_module_process_event:2037 failed
11-14 15:16:36.000   199  1902 E mm-camera-intf: mm_stream_streamon: ioctl VIDIOC_STREAMON failed: rc=-1
11-14 15:16:36.000   199  1902 E mm-camera-intf: mm_channel_start: start stream failed at idx(1)
11-14 15:16:36.000   255  1915 E mm-camera: mct_pipeline_process_set:command=8000009
11-14 15:16:36.000   255  1915 E mm-camera: mct_pipeline_process_set: stream_type = 7
11-14 15:16:36.010   199  1902 E mm-camera-intf: mm_stream_fsm_reg: invalid state (5) for evt (8), in(0x0), out(0x0)

	Lance（leave from Mozilla）
	Comment 84 • 10 years ago

This bug has been verified to fail on latest Flame 2.1, 2.2 build.

Issue steps:
1. Launch Loop client app and establish video call between Flame
2. During video call select Home page
3. Now launch Camera app and observe black camera preview
4. After 5sec. bring back the on-going video call
5. Terminate the call
6. Go back to camera app
7. Observe the preview is still black

Issue:
Flame 2.1: 
3 & 7: Preview is blank and control button doesn't work.

Flame 2.2:
3: Preview is blank and control button doesn't work.
7: Preview is normally, but control button doesn't work.

Note:After you kill camera process, launch it again, use normally.

Expected behaviors:
1. During video call should not allow user to launch camera app
2. If user able to launch camera app, after closing VT session, camera preview resume

Reproducing rate:
Flame 2.1: 4/10
Flame 2.2: 2/10

See attachment(Flame 2.1): 1255.mp4 and logcat_1255.txt
Found time: About:12:55
Loop version: bd8f1c2

Flame 2.1 version:
Gaia-Rev        9df43e6e696da43b6a1433283dc5b155987747bc
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/095a46f32cd4
Build-ID        20150129003733
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150129.041104
FW-Date         Thu Jan 29 04:11:14 EST 2015
Bootloader      L1TC000118D0

Flame 2.2 version:
Gaia-Rev        6e494f1d2676d231abba7dcc2e2822d1170d2d02
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5e6fac01a72f
Build-ID        20150129003432
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150129.042943
FW-Date         Thu Jan 29 04:29:53 EST 2015
Bootloader      L1TC000118D0

	Lance（leave from Mozilla）
	Comment 86 • 10 years ago

Clone bug 1127761 to track this bug.

	Eric Chang [:ericcc] [:echang]
	Comment 87 • 10 years ago

remove ni, we have a bug 1127761 to track.

	Lance（leave from Mozilla）
	Comment 88 • 10 years ago

Please track the new bug 1127761, and we deleted the verifyme.

	Justin D'Arcangelo [:justindarc]
	Comment 89 • 9 years ago

Clearing NI? since this has been resolved.