Closed Bug 890427 Opened 7 years ago Closed 7 years ago

[Camera] repetitive touches makes camera app hang

Categories

(Firefox OS Graveyard :: Gaia::Camera, defect, P1, blocker)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, firefox24 wontfix, firefox25 wontfix, firefox26 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

RESOLVED FIXED
1.1 QE4 (15jul)
blocking-b2g leo+
Tracking Status
firefox24 --- wontfix
firefox25 --- wontfix
firefox26 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: leo.bugzilla.gaia, Assigned: djf)

References

Details

(Whiteboard: [TD-55218])

Attachments

(3 files, 4 obsolete files)

1. Title :  repetitive touches makes camera app hang
2. Precondition : Camera app should be working
3. Tester's Action : 1. Open camera app -> touch fast and repetitively on toggle and capture buttons randomly
                   
 
4. Detailed Symptom (ENG.) : camera app hangs with focusing ring shown and UI is disabled
5. Expected : camera app should not hang.

6.Reproducibility: Y
1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced
8.Gaia Revision: 65e861f5941cab66815a2f79b0bc0a6a0fc8f6fe
9.Personal email id: gjyothiprasad@gmail.com
blocking-b2g: --- → leo+
Priority: -- → P1
Target Milestone: --- → 1.1 QE4 (15jul)
Attached file Video attachment (obsolete) —
Attached video shows camera app got hanged after random repetitive touches on toggle and capture buttons.
Whiteboard: [TD-55218]
Confirmed--I have been able to reproduce on Inari:
- gecko: b2g18:45507bc2bea1
- gaia: v1-train:7c40bdaeaffae708342fc773926dcfac5389348e

It took a few attempts to reproduce, but the hang (symptoms: black viewfinder, gallery button enabled, shutter and video-mode switch buttons disabled, tapping the viewfinder causes the filmstrip to come and go) _seems_ to happen after switching to video mode, hitting the record button twice in rapid succession to start and stop recording, and then switching back to photo mode.
This is definitely blocking: once the camera gets into this state, it is stuck. Closing and reopening the camera does not clear the hang. The lockscreen camera is also hosed.
Assignee: nobody → mhabicher
Target date for this issue in leo device: July 12, 2013.
The CameraThread is blocked here:

Thread 14 (Thread 459.476):
#0  __ioctl () at bionic/libc/arch-arm/syscalls/__ioctl.S:9
#1  0x40053b5c in ioctl (fd=<value optimized out>, request=8) at bionic/libc/bionic/ioctl.c:41
#2  0x42257a6e in android::IPCThreadState::talkWithDriver (this=0x425ffd40, doReceive=true)
    at frameworks/base/libs/binder/IPCThreadState.cpp:804
#3  0x42257e0c in android::IPCThreadState::waitForResponse (this=0x425ffd40, reply=0x44cffd68, acquireResult=0x0)
    at frameworks/base/libs/binder/IPCThreadState.cpp:668
#4  0x4225846c in android::IPCThreadState::transact (this=0x425ffd40, handle=2, code=5, data=..., reply=0x44cffd68, flags=16)
    at frameworks/base/libs/binder/IPCThreadState.cpp:560
#5  0x4225549e in android::BpBinder::transact (this=0x437c9580, code=5, data=..., reply=0x44cffd68, flags=0)
    at frameworks/base/libs/binder/BpBinder.cpp:165
#6  0x42892422 in android::BpCamera::unlock (this=0x42255479) at frameworks/base/libs/camera/ICamera.cpp:264
#7  0x4288fe9e in android::Camera::setPreviewTexture (this=<value optimized out>, surfaceTexture=<value optimized out>)
    at frameworks/base/libs/camera/Camera.cpp:196
#8  0x41ec6a0c in ?? () from /home/mikeh/dev/mozilla/btg024/objdir-gecko-b2g18-debug/dist/bin/libxul.so
#9  0x41ec6a0c in ?? () from /home/mikeh/dev/mozilla/btg024/objdir-gecko-b2g18-debug/dist/bin/libxul.so

...which is called from here: http://mxr.mozilla.org/mozilla-b2g18/source/dom/camera/GonkCameraHwMgr.cpp#186

This launches a binder call into:
http://androidxref.com/4.0.4/xref/frameworks/base/libs/camera/Camera.cpp#187

...and into the Camera service here:
http://androidxref.com/4.0.4/xref/frameworks/base/services/camera/libcameraservice/CameraService.cpp#570

...then to ::setPreviewWindow():
http://androidxref.com/4.0.4/xref/frameworks/base/services/camera/libcameraservice/CameraService.cpp#515

Hardware calls are here:
http://androidxref.com/4.0.4/xref/frameworks/base/services/camera/libcameraservice/CameraService.cpp#536

...and here:
http://androidxref.com/4.0.4/xref/frameworks/base/services/camera/libcameraservice/CameraService.cpp#541
Okay, got it. This bug is a deadlock between the main mediaserver thread and the camera snapshot thread.

1. call takePicture(), which sets a snapshot mode flag in the camera library, and return
2. call startPreview(), which sees the snapshot mode flag is set, and so blocks on a condvar waiting for the snapshot mode flag to get cleared
3. the camera library invokes the takePicture callback

Normally, the snapshot mode flag is cleared in (3).  In order to do this, the callback needs to get the CameraService::Client::mLock, but the mLock is still held by the blocked takePicture() call in (1).
The solution is (probably) to disable all of the non-preview messages when starting the preview. Then, once the preview is started, enable messages like shutter and compressed-image.
Ugh, the CameraService API doesn't allow us to specify which messages are enabled (aside from takePicture(), for some reason).  Nor does it expose an API where we can cancel picture-taking (though we can cancel auto-focus--again, for some reason).

Sotaro, any thoughts on the best approach to tackling this? I'm thinking of maintaining a counter to keep track of outstanding API calls that invoke callbacks, and deferring any API calls until we receive all of those callbacks.
Flags: needinfo?(sotaro.ikeda.g)
I also regenerate the freeze and got following stack.

(gdb) info stack
#0  __ioctl () at bionic/libc/arch-arm/syscalls/__ioctl.S:9
#1  0x4008e280 in ioctl (fd=<value optimized out>, request=8) at bionic/libc/bionic/ioctl.c:41
#2  0x41781a6e in android::IPCThreadState::talkWithDriver (this=0x4385fe20, doReceive=true)
    at frameworks/base/libs/binder/IPCThreadState.cpp:804
#3  0x41781e0c in android::IPCThreadState::waitForResponse (this=0x4385fe20, reply=0x44333dd0, acquireResult=0x0)
    at frameworks/base/libs/binder/IPCThreadState.cpp:668
#4  0x4178246c in android::IPCThreadState::transact (this=0x4385fe20, handle=2, code=5, data=..., reply=0x44333dd0, 
    flags=16) at frameworks/base/libs/binder/IPCThreadState.cpp:560
#5  0x4177f49e in android::BpBinder::transact (this=0x43f0f800, code=5, data=..., reply=0x44333dd0, flags=0)
    at frameworks/base/libs/binder/BpBinder.cpp:165
#6  0x41dc4486 in android::BpCamera::startPreview (this=<value optimized out>)
    at frameworks/base/libs/camera/ICamera.cpp:109
#7  0x41dc1f02 in android::Camera::startPreview (this=<value optimized out>) at frameworks/base/libs/camera/Camera.cpp:206
#8  0x40855cd0 in android::GonkCameraHardware::CancelAutoFocus (this=0x43fb84c0)
    at /home/sotaro/b2g_11_leo/B2G/gecko/dom/camera/GonkCameraHwMgr.cpp:267
#9  0x40854ae0 in mozilla::nsGonkCameraControl::SetParameter (this=0x43860d40, aKey=<value optimized out>, aValue=1)
    at /home/sotaro/b2g_11_leo/B2G/gecko/dom/camera/GonkCameraControl.cpp:629
#10 0x40be252e in nsCOMPtr<nsIThreadObserver>::operator= (this=0x42c9fe80, mayWait=<value optimized out>, 
    result=0x42c9fe80) at ../../dist/include/nsCOMPtr.h:614
#11 nsThread::ProcessNextEvent (this=0x42c9fe80, mayWait=<value optimized out>, result=0x42c9fe80)
    at /home/sotaro/b2g_11_leo/B2G/gecko/xpcom/threads/nsThread.cpp:630
#12 0x40bc2906 in NS_ProcessPendingEvents_P (thread=0x43fb84c0, timeout=1140033984)
    at /home/sotaro/b2g_11_leo/B2G/objdir-gecko/xpcom/build/nsThreadUtils.cpp:171
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Mike Habicher [:mikeh] from comment #8)
> Ugh, the CameraService API doesn't allow us to specify which messages are
> enabled (aside from takePicture(), for some reason).  Nor does it expose an
> API where we can cancel picture-taking (though we can cancel
> auto-focus--again, for some reason).
> 
> Sotaro, any thoughts on the best approach to tackling this? I'm thinking of
> maintaining a counter to keep track of outstanding API calls that invoke
> callbacks, and deferring any API calls until we receive all of those
> callbacks.

I feel it is better to make clear about the camera hw's limitation. We do not have information about it. We do not have enough src/symbol around camera module. We cannot debug around it. It is better to get the info from partners.
(In reply to Mike Habicher [:mikeh] from comment #8)
> Sotaro, any thoughts on the best approach to tackling this? I'm thinking of
> maintaining a counter to keep track of outstanding API calls that invoke
> callbacks, and deferring any API calls until we receive all of those
> callbacks.

I am not sure that the counter is a good choice to solve this problem. But a similar thing seems necessary. It seems necessary to wait a snapshot completion. If camera is in SNAPSHOT_IN_PROGRESS, Camera API need to reject camera control. In android, the camera app checks if the camera hw is idle or not. And the app decide how to control the camera hw.
http://androidxref.com/4.0.4/xref/packages/apps/Camera/src/com/android/camera/Camera.java#isCameraIdle
I do not have enough symbol in mediaserver side. Though following seems the place of the deadlock in my case.

 (gdb) info stack
#0  __futex_syscall3 () at bionic/libc/arch-arm/bionic/atomics_arm.S:182
#1  0x400fe544 in _normal_lock (mutex=0xab0dbc) at bionic/libc/bionic/pthread.c:951
#2  pthread_mutex_lock (mutex=0xab0dbc) at bionic/libc/bionic/pthread.c:1041
#3  0x40d44062 in android::QCameraStream_preview::initDisplayBuffers() ()
   from /home/sotaro/b2g_11_leo/B2G/out/target/product/leo/system/lib/hw/camera.msm7627a.so
#4  0x40d44062 in android::QCameraStream_preview::initDisplayBuffers() ()
   from /home/sotaro/b2g_11_leo/B2G/out/target/product/leo/system/lib/hw/camera.msm7627a.so
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
Hi Mike,

Do you have updates on this issue?
Flags: needinfo?(mhabicher)
I am working on this issue. I should have a patch ready for testing by EoD today/early tomorrow.
Flags: needinfo?(mhabicher)
Incidentally, I've refined the STR for this bug:
1. open the Camera app
2. press the Mode switch button to switch to Video mode
3. press the Mode switch button again and IMMEDIATELY start pressing the shutter button rapidly to try to take a picture.

Expected behaviour: the camera should take a picture.

Observed results: the camera service deadlocks as described in comment 6.
From an API perspective, I think the problem is this:
1. switching to the picture mode disables the camera UI
2. the Camera app calls getPreviewStream() which returns right away
3. the onsuccess callback passed to getPreviewStream() is invoked and connects the stream to the viewfinder <video> DOM object, then enables camera UI

Connecting the preview stream object to the <video> tag triggers the call to startPreview() on the camera hardware, and this happens asynchronously. If this happens after the UI is enabled, a number of problems can occur:
a. we run the risk of deadlocking the camera service as in comment 6
b. the call to takePicture() will fail

I can protect the CameraControl API against deadlocking the camera service, but the Camera app will need to prevent enabling the UI until the preview stream has started.

daleharvey, is there a property on the video tag the Camera app can listen for? I see 'onreadystatechange', but not sure if that's the right way to go.
Flags: needinfo?(dale)
HI MIke and Dale,

Addition of minimal delay when preview switching (in changeMode()) prevents this scenario. Also, addition of delay will not cause any issues in performance.

setTimeout(function() {
      if (self._captureMode === self.CAMERA) {
        self._cameraObj.getPreviewStream(self._previewConfig,
                 gotPreviewStream.bind(self));
      } else {
        self._previewConfigVideo.rotation = self._phoneOrientation;
        self._cameraObj.getPreviewStreamVideoMode(self._previewConfigVideo,
                 gotPreviewStream.bind(self));
      }
    }, 50);

Mike, I have tried disabling UI till next preview loads, but it didn't work few times, though I need to verify my implementaion
There are no events fired on the video object when mozSrcObject is set, this means we cant disable UI until its properly loaded, the setTimeout as a hack can work intermittently, but its asking for further bugs

Can we get the video tag (or the camera api) to fire and event when the UI is ready to be enabled?
Flags: needinfo?(dale)
(In reply to Dale Harvey (:daleharvey) from comment #18)
>
> There are no events fired on the video object when mozSrcObject is set, this
> means we cant disable UI until its properly loaded, the setTimeout as a hack
> can work intermittently, but its asking for further bugs

The setTimeout() hack proves the solution, but I agree it's too non-deterministic.

> Can we get the video tag (or the camera api) to fire and event when the UI
> is ready to be enabled?

Yes, I can add something like an onPreviewStarted callback to CameraControl.

Actually, since you'll probably want to disable the UI when the preview stops, I'll make it onPreviewStateChanged(bool running) (or something like that), and camera.js can react appropriately.

(I'm a little miffed at how rigid the AOSP API is; but this exercise has given me some things to think about in terms of the future migration to the getUserMedia() API.)
sotaro, this patch adds a |onPreviewStateChange| attribute to CameraControl that invokes a callback function with an argument of either "started" or "stopped", so that the Camera app can enable/disable its UI as needed.

I'm on PTO as of this evening, no laptop (but should be able to check email), so I'm hoping you can look this over and ask someone (jst?--it's almost completely boilerplate) to review it, then land it. (Please.)

I've tested the patch and it works as expected. As follow-up, daleharvey will need to make the corresponding gaia change to hook into the new callback.
Attachment #771525 - Attachment is obsolete: true
Attachment #777368 - Flags: feedback?(sotaro.ikeda.g)
This is the m-c version of the patch; it uses a different macro to indicate unreachable code.
Attachment #777424 - Flags: feedback?(sotaro.ikeda.g)
Attached patch HOWTO (obsolete) — Splinter Review
And this is how to use it (kind of).
Attachment #777426 - Flags: feedback?(dale)
mikeh, okey. I am going to handle the patches. Have a nice PTO!
That looks good to me, sotoro do you want to take this bug, I will get the gaia side of it done today or tomorrow so they can be tested together
Attachment #777426 - Flags: feedback?(dale) → feedback+
(In reply to Dale Harvey (:daleharvey) from comment #25)
> That looks good to me, sotoro do you want to take this bug, I will get the
> gaia side of it done today or tomorrow so they can be tested together

I take this bug. Thanks!
Assignee: mhabicher → sotaro.ikeda.g
Comment on attachment 777368 [details] [diff] [review]
[v1][b2g18] Let the Camera app know when the preview has actually started/stopped

Looks good!
Attachment #777368 - Flags: feedback?(sotaro.ikeda.g) → feedback+
Comment on attachment 777424 [details] [diff] [review]
[v1][m-c] Let the Camera app know when the preview has actually started/stopped

lgtm.
Attachment #777424 - Flags: feedback?(sotaro.ikeda.g) → feedback+
Mike,

Camera app hangs with focus ring during the following scenario also:
1. touch gallery -> immediately touch capture -> camera app hangs with focus ring
2. touch capture button -> immediately press home button -> camera app hangs with focus ring.
Flags: needinfo?(mhabicher)
Hi Leo, can you get a stack trace of where the deadlock happen by using GDB?

It seems like deadlock happens somewhere by the contention between taking picture and camera. Bug 836427 seems similar bug, but different result.
Flags: needinfo?(leo.bugzilla.gaia)
(In reply to Sotaro Ikeda [:sotaro] from comment #30)
> It seems like deadlock happens somewhere by the contention between taking
> picture and camera. Bug 836427 seems similar bug, but different result.

If it is correct, it is better to handle as another bug. FIX should be different.
(In reply to Leo from comment #29)
> Camera app hangs with focus ring during the following scenario also:
> 1. touch gallery -> immediately touch capture -> camera app hangs with focus
> ring
> 2. touch capture button -> immediately press home button -> camera app hangs
> with focus ring.

Is it Bug 895857? If it is, discuss there :-)
(In reply to Sotaro Ikeda [:sotaro] from comment #30)
> Hi Leo, can you get a stack trace of where the deadlock happen by using GDB?

I checked that on my leo, dead lock did not happend in camera app process, but happened only on camera service side. I do not have a symbol around camera hw. I can not investigate more about the place of the deadlock.

We should move the discussion of Comment 29 case to Bug 895857.
Flags: needinfo?(leo.bugzilla.gaia)
Flags: needinfo?(leo.bugzilla.gaia)
(In reply to Sotaro Ikeda [:sotaro] from comment #33)
> (In reply to Sotaro Ikeda [:sotaro] from comment #30)
> > Hi Leo, can you get a stack trace of where the deadlock happen by using GDB?
> 
> I checked that on my leo, dead lock did not happend in camera app process,
> but happened only on camera service side. I do not have a symbol around
> camera hw. I can not investigate more about the place of the deadlock.
> 
> We should move the discussion of Comment 29 case to Bug 895857.

Yes, we can discuss on this issue issue in Bug 895857
Flags: needinfo?(leo.bugzilla.gaia)
Comment on attachment 777368 [details] [diff] [review]
[v1][b2g18] Let the Camera app know when the preview has actually started/stopped

jst, can you review a patch?
Attachment #777368 - Flags: review?(jst)
Attachment #777424 - Flags: review?(jst)
Dale, about when can we get the gaia side?
Flags: needinfo?(dale)
Dale,

Feel free to resassign to me if you're busy with other things.
Comment on attachment 777368 [details] [diff] [review]
[v1][b2g18] Let the Camera app know when the preview has actually started/stopped

Looks good, r=jst
Attachment #777368 - Flags: review?(jst) → review+
Attachment #777424 - Flags: review?(jst) → review+
Any chance we could get an automated test for this fix?
mikeh, do you know about current status of camera's automated test?
Flags: needinfo?(mhabicher)
Cheers djf, yeh given I am not involved in planning and that it impacts everyone estimations etc I think in general its best to start sticking to the teams by default, of course if your needing some more resources then feel free to assign me.
Assignee: sotaro.ikeda.g → dflanagan
Flags: needinfo?(dale)
I'm not able to reproduce this bug on my unagi, and my Leo device has such old firmware that it thinks it has no device storage, so it won't run the camera at all...

I can try making a Gaia patch, but it looks like I'll need someone else to test it.
David, 

I can help to do the test. I had tested with my Inari which can reproduce this bug. But I may need more information on how to do it.
I also can help the test. I can reproduce it on leo device.

Following is easier to reproduce the problem for me.
STR
- [1] Start camera app
- [2] Change to video mode
- [3] When you tap a camera mode icon, you also tap camera shutter button slightly later than the camera mode icon.

Frequency Rate: 1/20

When doing [3], I tap shutter button multiple times not to fail taking a photo.
Sotaro,

If I'm understanding the bug correctly, this patch should be the fix that is required in Gaia. But since I can't reproduce it reliably, I need you to test it.  Note that I've only attempted to use the new onPreviewStateChange callback in the case where the user switches from photos to videos or vice versa.  If you know of other cases where deadlock can arise from the same underlying bug, let me know.

John: your testing is also welcome, but I am also asking you to review the patch.
Attachment #784600 - Flags: review?(johu)
Attachment #784600 - Flags: feedback?(sotaro.ikeda.g)
Djf, thanks for the patch. I will check the patch on leo device soon.
When I applied attachment 784600 [details], camera app does not work correctly. When I changed the camera app to video mode, camera app's all buttons stay disabled and never enabled again.
(In reply to Sotaro Ikeda [:sotaro] from comment #47)
> When I applied attachment 784600 [details], camera app does not work
> correctly. When I changed the camera app to video mode, camera app's all
> buttons stay disabled and never enabled again.

Corection: shutter button and mode button stay disabled.
To help make this bug easy to happen, I add the following code to the end of enableButtons at camera.js:

    window.setTimeout(function(){
      this.capturePressed({'target': this.captureButton});  
    }.bind(this));

That help me press the button once it is enabled. After the patch, the STR of this bug changes to:
1. open camera app
2. once shutter sound heard, press home
3. do 1 and 2 repeatedly.

The reason to press home and reopen is that the camera app stops preview while invisible and starts preview while visible. The program above helps press shutter button ASAP. This is made according to the comment 6 and comment 16. We may have the halt screen about 2 or 3 times. After the halt screen, we may need to kill camera app and reopen it to double confirm that.
Comment on attachment 784600 [details]
Gaia portion of the patch

This patch looks good. But we may need to add the similar code to the function of loadCameraPreview. It's about line 843. There are two functions calling getPreviewStream. One is at the place this patch is. Another is at loadCameraPreview. With the testing code I propose, it is still reproducible. I think the reason is startPreview function uses loadCameraPreview.
Attachment #784600 - Flags: review?(johu) → review-
(In reply to Sotaro Ikeda [:sotaro] from comment #48)
> (In reply to Sotaro Ikeda [:sotaro] from comment #47)
> > When I applied attachment 784600 [details], camera app does not work
> > correctly. When I changed the camera app to video mode, camera app's all
> > buttons stay disabled and never enabled again.
> 
> Corection: shutter button and mode button stay disabled.

Sotaro: did you have Mike's gecko patch applied when you tested? The behavior you describe is what I'd expect if you tested the gaia patch against an unpatched gecko.
This is one of those bugs where we'll have to be sure that that gecko patch has landed before we apply the gaia patch.
(In reply to John Hu [:johnhu] from comment #49)
> To help make this bug easy to happen, I add the following code to the end of
> enableButtons at camera.js:
> 
>     window.setTimeout(function(){
>       this.capturePressed({'target': this.captureButton});  
>     }.bind(this));
> 

Brilliant!  Thanks for that idea.  I'll patch loadCameraPreview too.  Were you able to verify that the patch did at least fix the deadlock when switching from video to photo and then pressing the shutter button right away?
I had tried but it was too hard to reproduce. The reproducing rate is pretty low. I had reproduced twice yesterday. But I never reproduce it today, no matter the patch applied or not.

But, I had patched loadCameraPreview. The automatic testing code can't reproduce it. I think the code and logic is correct.
I found another testing code to test switch mode:

  enableButtons: function camera_enableButtons() {
    this.captureButton.removeAttribute('disabled');
    this.switchButton.removeAttribute('disabled');

    if (this._pendingPick) {
      this.cancelPickButton.removeAttribute('disabled', 'disabled');
    }
    // add here
    window.setTimeout(function(){
      if (this._captureMode === this.CAMERA) {
        this.capturePressed({'target': this.captureButton});
      }
      this.toggleModePressed({'target': this.switchButton})
    }.bind(this));
    // end of testing code
  },


This code help us switch mode and press shutter button when buttons are enabled. The STR with this code is:
1. open camera app
2. wait shutter sound
3a. when no more shutter sound heard, kill and reopen camera app for double confirm. If it only shows the shutter button, it is affected. If it shows 3 buttons, and we will need to redo the test.
3b. If we heard more than 10 times, kill camera app and redo the test.


The reproducing ratio is about 1/5. If we dump the logcat lively, we may found the crash message of media server when no more shutter sound heard.

BTW, I had tested the patch made by David. The camera app works. It will take picture until SD card is full.
Another thing: we should be able to take pictures forever with the above testing code. But it doesn't. Camera app takes about 50 pictures and stops. That's why we need to restart the camera app for double confirm.

After the patch, it still can't take picture forever. And the symptom looks the same 30 pictures and stopped. But we can't find crash message in the logcat in this time. I think there may be another race condition inside Camera app and mostly likely in gaia part.
Comment on attachment 784600 [details]
Gaia portion of the patch

John,

I've updated the pull request to use onPreviewStateChange in loadCameraPreview(). Thanks for pointing that out.

As part of this change, I modified the _camera property of the Camera object to be called _cameraNumber. This makes the code a lot clearer, I think.

Does this fix the bug for you?
Attachment #784600 - Flags: review- → review?(johu)
Comment on attachment 784600 [details]
Gaia portion of the patch

Thanks, David.

This version works good for both cases. And the change to cameraNumber makes sense. I was also confused about it before. r=me, 9eadf36.
Attachment #784600 - Flags: review?(johu) → review+
(In reply to David Flanagan [:djf] from comment #51)
> Sotaro: did you have Mike's gecko patch applied when you tested? The
> behavior you describe is what I'd expect if you tested the gaia patch
> against an unpatched gecko.
> This is one of those bugs where we'll have to be sure that that gecko patch
> has landed before we apply the gaia patch.

Sorry, I totally forgotten about it. I re-confirm by new gaia patch soon.
I applied attachment 784600 [details] to master leo. This time I applied also gecko side patch. Mode change works correctly, but camera preview does not resume after taking a photo.
I am going to check more about Comment 59. Though I am now recreating the built from scrash :-(
(In reply to Sotaro Ikeda [:sotaro] from comment #59)
> I applied attachment 784600 [details] to master leo. This time I applied
> also gecko side patch. Mode change works correctly, but camera preview does
> not resume after taking a photo.

It is not related to the bug. Default Master LEO have the problem might be affected by Bug 858914, Bug 900012.
master leo can not used for testing from Comment 61. I checked the patches on master hamachi. STR in Comment 44 seems OK. But I find a problem in following STR. I am not sure it happened by same problem on this bug and if it is master dependent problem.

STR
-[1] Start Camera app
-[2] Change to video mode
-[3] start video recording and stop recording some seconds after the start.
-[4] soon change to camera mode and take a picture.
gecko's master have a problem around camera. So I checked STR in Comment 44 by gecko(b2g18) + gaia(master) on leo and hamachi. No problem for the STR.
(In reply to Sotaro Ikeda [:sotaro] from comment #62)
> STR
> -[1] Start Camera app
> -[2] Change to video mode
> -[3] start video recording and stop recording some seconds after the start.
> -[4] soon change to camera mode and take a picture.

The above STR did not happened on b2g18 gecko on leo and hamachi. I am going to create a bug for the STR.
Attachment #784600 - Flags: feedback?(sotaro.ikeda.g) → feedback+
Sotaro,

Do you think we should land the gecko and gaia patches, since they seem to fix the bug for 1.1, or do you think we need to investigate further and figure out what is happening on master?

Either way, I have re-assigned the bug back to you:

1) If you think we should land the patch, I don't know how to land gecko patches, and I don't have commit rights on m-c. If you land the gecko patch today, I'll land the gaia patch on monday to avoid people pulling gaia with an out of date gecko.

2) If you think we should keep investigating, it sounds like there are gecko issues that I won't be able to help with. In this case maybe we should re-assign it back to Mike, but since he is still on vacation, you get it for now.
Assignee: dflanagan → sotaro.ikeda.g
Master has a lot of problems around graphics, camera and video. They can not be fixed until Bug 858914 is fixed. I am going to create a new bug and going to set blocked by the bug.

I always ask to check-in by using keyword. I am going to do it after passed try server test.
Attachment #777426 - Attachment is obsolete: true
Update the patch's header. Carry "jst: review+".
Attachment #777368 - Attachment is obsolete: true
Attachment #785316 - Flags: review+
Update the patch's header. Carry "jst: review+".
Attachment #777424 - Attachment is obsolete: true
Adding "KEEP OPEN" to the white board. Please don't close the bug when the gecko patch is checked in. There is a corresponding gaia patch that needs to land after the gecko patch has landed and made it to people's nightly builds.
Whiteboard: [TD-55218] → [TD-55218] [KEEP OPEN]
https://hg.mozilla.org/integration/b2g-inbound/rev/4eef02979648

[leave open] is the magic incantation you're looking for :)
Keywords: checkin-needed
Whiteboard: [TD-55218] [KEEP OPEN] → [TD-55218][leave open]
Comment on attachment 785317 [details] [diff] [review]
[v2][m-c] Let the Camera app know when the preview has actually

Carry "jst: review+".
Attachment #785317 - Flags: review+
Can the patch be uplifted to gecko v1-train?
Keywords: checkin-needed
Flags: needinfo?(mhabicher)
https://hg.mozilla.org/releases/mozilla-b2g18/rev/1433d653b701

Please resolve the bug and set the appropriate status flags when landing the Gaia patch on the appropriate branches.
Keywords: checkin-needed
Whiteboard: [TD-55218][leave open] → [TD-55218]
Blocks: 898326
djf, gecko side was committed. I assign you for remaining gaia side.
Assignee: sotaro.ikeda.g → dflanagan
https://github.com/mozilla-b2g/gaia/commit/376d45c7afdbf97f75d55803e15fbb87931f3a50
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x  376d45c7afdbf97f75d55803e15fbb87931f3a50
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(dflanagan)
Weird. The needinfo above does not show up on my dashboard. Maybe because the bug is closed?
Flags: needinfo?(dflanagan)
Uplifted to v1-train: https://github.com/mozilla-b2g/gaia/commit/3335e3ba4e8bb6965112514f00e8ba4e312ab540

I don't know if anyone is still using the v1-train branch, but even if not, this revised patch will be easier to cherry-pick to the hd branch.

needinfo jhford in case he needs to know that this can now be uplifted for the hd branch.
Flags: needinfo?(jhford)
v1.1.0hd: 3335e3ba4e8bb6965112514f00e8ba4e312ab540
v1.1.0hd: 9df4b6b2e06b0c72611e20b43c321ee89742fd70
(In reply to David Flanagan [:djf] from comment #81)
> needinfo jhford in case he needs to know that this can now be uplifted for
> the hd branch.

The HD Branch team is taking care of the v1-train to v1.1.0hd branch merges, and it was completed :)
Flags: needinfo?(jhford)
You need to log in before you can comment on or make changes to this bug.