839628 - Camera - emulator - MediaStream isn't getting frames from camera, preview stays black, deadlock?

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: gdb - info threads 17 (camera driver) and 16 (MediaStream)

This is a new bug, which seems to be different from the symptoms reported in bug 826072.

STR:
0. ./config.sh emulator && ./build.sh && ./run-emulator.sh
1. unlock the lock screen
2. click on the camera app icon

Expected:
Even if the preview doesn't really work, the camera should make a good attempt at starting up.

Observed:
In the logcat, the camera __data_cb calls dequeueBuffer() four times then stops; if camera debugging is turned on, "Try again" appears, indicating that the camera is waiting for buffers to become available (as they would be, if the MediaStream thread were consuming them).

Comment 1

[Mike Habicher [:mikeh] (high bugzilla latency)]

logcat output with no camera debugging:

D(  178:0xb2) Emulated camera list: 
D(  178:0xb2) Initialize: Fake camera is facing back
V(  178:0xb2) 1 cameras are being emulated. Fake camera ID is 0
V(  178:0xb2) getCameraInfo: id = 0
V(  178:0xb2) getCameraInfo
I(  178:0xc5) Opening camera 0
V(  178:0xc5) cameraDeviceOpen: id = 0
V(  178:0xc5) connectCamera
V(  178:0xc5) connectDevice
V(  178:0xc5) getCameraInfo: id = 0
V(  178:0xc5) getCameraInfo
V(  178:0xc5) setCallbacks(0)
V(  178:0xc5) setCallbacks: 0x40b364c9, 0x40b366cd, 0x40b36615, 0x40b374a5 (0x4433d100)
V(  178:0xc5) getParameters(0)
V(  178:0xc5) setParameters(0)
V(  178:0xc5) setParameters
D(  178:0xc5) === Value changed: preview-frame-rate: 24 -> 30
V(  178:0xc5) getParameters(0)
V(  178:0xc5) setParameters(0)
V(  178:0xc5) setParameters
D(  178:0xc5) === Value changed: preview-size: 640x480 -> 352x288
E(  178:0xb2) [JavaScript Error: "TypeError: this._pictureStorage.stat is not a function" {file: "app://camera.gaiamobile.org/js/camera.js" line: 711}]
V(  178:0xc5) setPreviewWindow(0) buf 0x443ce808
V(  178:0xc5) setPreviewWindow &mHalPreviewWindow 0x4433d110 mHalPreviewWindow.user 0x4433d100
V(  178:0xc5) setPreviewWindow: current: 0x0 -> new: 0x4433d110
V(  178:0xc5) enableMsgType(0)
V(  178:0xc5) enableMessage: msg_type = 0x10
V(  178:0xc5)     CAMERA_MSG_PREVIEW_FRAME
V(  178:0xc5) **** Currently enabled messages:
V(  178:0xc5)     CAMERA_MSG_PREVIEW_FRAME
V(  178:0xc5) startPreview(0)
V(  178:0xc5) doStartPreview
V(  178:0xc5) startPreview
D(  178:0xc5) Starting camera: 352x288 -> NV21(yuv420sp)
V(  178:0xc5) startDevice
V(  178:0xc5) commonStartDevice: Allocated 0x4468c000 152064 bytes for 101376 pixels in NV21[352x288] frame
V(  178:0xc5) startDeliveringFrames
V(  178:0xc5) startWorkerThread
V(  178:0xc7) Starting emulated camera device worker thread...
V(  178:0xc7) Emulated device's worker thread has been started.
V(  178:0xc7) onNextFrameAvailable: Adjusting preview windows 0x4433d110 geometry to 352x288
V(  178:0xc7) __data_cb
V(  178:0xc7) __data_cb
V(  178:0xc7) __data_cb
V(  178:0xc7) __data_cb

Comment 2

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: logcat - showing the deadlock

With |NSPR_LOG_MODULES=Camera:5,MediaStreamGraph:5|

230: dequeueBuffer: returning slot=0 buf=43c5d340
240: queueBuffer
249: CameraGraphicBuffer::CameraGraphicBuffer
252: __data_cb
259: writing video frame 4436f6c0
  ...
277: dequeueBuffer: returning slot=1 buf=43c5d460
294: queueBuffer
296: CameraGraphicBuffer::CameraGraphicBuffer
299: __data_cb
319: writing video frame 44b543c0
  ...
332: returnBuffer: slot=0
333: CameraGraphicBuffer::~CameraGraphicBuffer
  ...
340: dequeueBuffer: returning slot=0 buf=43c5d340
350: queueBuffer
352: CameraGraphicBuffer::CameraGraphicBuffer
362: __data_cb
375: writing video frame 4436f680
  ...
395: returnBuffer: slot=1
396: CameraGraphicBuffer::~CameraGraphicBuffer
  ...
403: dequeueBuffer: returning slot=1 buf=43c5d460
413: queueBuffer
415: CameraGraphicBuffer::CameraGraphicBuffer
418: __data_cb
438: writing video frame 44b54380
  ...
441: dequeueBuffer: Try again

At this point, returnBuffer() is never called again, and the camera library is blocked in dequeueBuffer().  The MediaStream thread continues to run.

One key difference between the emulated camera and the unagi driver is that the emulated camera seems to only use two preview frame buffers.

Comment 3

[Mike Habicher [:mikeh] (high bugzilla latency)]

Yep, the emulator camera isn't allocating enough buffers to keep MediaStream fed.  Bumping GonkNativeWindow::MIN_UNDEQUEUED_BUFFERS up to (e.g.) 9 from it's default of 2 clears the logjam and lets the camera work--though taking a picture ultimately fails.

Comment 4

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: Increase GonkNativeWindow::MIN_UNDEQUEUED_BUFFERS from 2 to 4

With only 2 buffers, the MediaStream gets stuck; with 4, things work smoothly.

Comment 5

[Sotaro Ikeda [:sotaro]]

Comment on attachment 712004 [details] [diff] [review]
Increase GonkNativeWindow::MIN_UNDEQUEUED_BUFFERS from 2 to 4

Increase MIN_BUFFER_SLOTS is not a good idea. It increase memory use in devices(like unagi). The problem happens just because qemu's camera hal implementation is incorrect. If implementaion is correct, the problem does not happen.


A correct camera hal requests to allocate "PreviewBufferCount + NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS" number of buffers by calling GonkNativeWindow::setBufferCount(). But the qemu's camera hal do not call GonkNativeWindow::setBufferCount(). Then GonkNativeWindow allocates NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS buffers. And the camera hal tries to call GonkNativeWindow::dequeueBuffer() forever during preview and do not care about the buffer count. It is a incorrect behavior.

SurfaceTexture implements a workaround for this. The SurfaceTexture limits dequeued buffer number to one, when camera hal do not set the Buffercount.

I can provide a fix for this. Is it OK to provide fix for the problem by me?

SurfaceTexture's workaround
 - http://androidxref.com/4.0.4/xref/frameworks/base/libs/gui/SurfaceTexture.cpp#384

emulator's camera hal implementation
 - http://androidxref.com/4.0.4/xref/development/tools/emulator/system/camera/PreviewWindow.cpp

Comment 6

[Mike Habicher [:mikeh] (high bugzilla latency)]

sotaro, absolutely!--if you have a more correct solution, feel free to propose a patch.

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: patch: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

Add buffer count check to GonkNativeWindow::dequeueBuffer() so as not to try to dequeue too much buffers.
Confirmed on Unagi phone and emulator. Unagi phone works correctly as before.
Emulator do not show the preview frame, but can take a photo.

Comment 8

[[e-mail] [:onecyrenus]]

Sotaro should there be a reviewer on this patch ?

Comment 9

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> Created attachment 712539 [details] [diff] [review]
> patch: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()
> 

basic code of the patch is borrowed from andorid's SurfaceTexture.

http://androidxref.com/4.0.4/xref/frameworks/base/libs/gui/SurfaceTexture.cpp#393

Comment 10

[Sotaro Ikeda [:sotaro]]

(In reply to [e-mail]  [:onecyrenus] from comment #8)
> Sotaro should there be a reviewer on this patch ?

I am going to get reviewed by :kanru.

Comment 11

[Sotaro Ikeda [:sotaro]]

After applying the patch, I can take a photo on emulator, but still can not see preview. I think this is a different problem that relates to gralloc buffer on emulator.

Comment 12

[[e-mail] [:onecyrenus]]

My problem is solved if we can take photos and actually get a file we can store. 

I am ok not being able to see the preview

Comment 13

[Sotaro Ikeda [:sotaro]]

Comment on attachment 712539 [details] [diff] [review]
patch: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

kanru, can you review the patch? Thanks.

Comment 14

[Mike Habicher [:mikeh] (high bugzilla latency)]

Comment on attachment 712004 [details] [diff] [review]
Increase GonkNativeWindow::MIN_UNDEQUEUED_BUFFERS from 2 to 4

Marking obselete since sotaro's patch is the correct solution.

Comment 15

[Sotaro Ikeda [:sotaro]]

kanru, do you have a time to review?

Comment 16

[Sotaro Ikeda [:sotaro]]

Attachment: patch v2: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

fix -int(mSynchronousMode) adjustments. GonkNativeWindow always runs as if mSynchronousMode = true.

Comment 17

[Mike Habicher [:mikeh] (high bugzilla latency)]

Comment on attachment 714432 [details] [diff] [review]
patch v2: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

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

Looks good.

Comment 18

[Sotaro Ikeda [:sotaro]]

Comment on attachment 714432 [details] [diff] [review]
patch v2: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

The patch do not work on emulator. 

From v1, the pach changes a logic of available buffer number as in SurfaceTexture(mSynchronousMode=true). Because GonkNativeWindow works as (mSynchronousMode=true) in dequeueBuffer(). But there is a difference between SurfaceTexture and GonkNativeWindow. SurfaceTexture is a final destination of GraphicBuffer in android. But GonkNativeWindow is not a final destination of the buffers. Theyare sent to Compositor.

Comment 19

[Sotaro Ikeda [:sotaro]]

At most, 2 buffers could become outside of GonkNativeWindow. attachment 714432 [details] [diff] [review] tries to dequeue buffers until remaining buffers becomes one.

Old attachment 714432 [details] [diff] [review] is the correct implementation for GonkNativeWindow. It tries to dequeue buffers until remaining buffers becomes two.

Comment 20

[Sotaro Ikeda [:sotaro]]

Comment on attachment 712539 [details] [diff] [review]
patch: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

mikeh, it becomes clear that v1 patch is correct implementation for GonkNativeWindow. Can you review the patch again?

Comment 21

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 712539 [details] [diff] [review]
patch: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

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

Drive-by

Comment 22

[Mike Habicher [:mikeh] (high bugzilla latency)]

Comment on attachment 712539 [details] [diff] [review]
patch: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

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

::: dom/camera/GonkNativeWindow.cpp
@@ +302,5 @@
> +            // MIN_UNDEQUEUED_BUFFERS check below.
> +            if (renderingCount > 0) {
> +                // make sure the client is not trying to dequeue more buffers
> +                // than allowed.
> +                const int avail = mBufferCount - (dequeuedCount+1);

nit: (dequeuedCount + 1)

Comment 23

[Sotaro Ikeda [:sotaro]]

Attachment: patch v3: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

apply a comment. carry "kchen: review+"

Comment 24

[Sotaro Ikeda [:sotaro]]

https://tbpl.mozilla.org/?tree=Try&rev=4217435cdf66

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ceb97502eb31

Comment 26

[Sotaro Ikeda [:sotaro]]

Comment on attachment 715602 [details] [diff] [review]
patch v3: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Can not take photoes on emulator
Testing completed: Tested in Emulator and Unagui phone.
Risk to taking this patch (and alternatives if risky):  low risk
String or UUID changes made by this patch: none

Comment 27

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 715602 [details] [diff] [review]
patch v3: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

Approving for v1-train since that is where we build emulator from.

Comment 28

[Mike Habicher [:mikeh] (high bugzilla latency)]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/061acc09b713

Comment 30

[Ryan VanderMeulen [:RyanVM]]

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