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

RESOLVED FIXED in Firefox 22

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mikeh, Assigned: mikeh)

Tracking

unspecified
B2G C4 (2jan on)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

Attachments

(1 attachment, 5 obsolete attachments)

Created attachment 711953 [details]
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).
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
Created attachment 711972 [details]
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.
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.
Assignee: nobody → mhabicher
Created attachment 712004 [details] [diff] [review]
Increase GonkNativeWindow::MIN_UNDEQUEUED_BUFFERS from 2 to 4

With only 2 buffers, the MediaStream gets stuck; with 4, things work smoothly.
Attachment #711953 - Attachment is obsolete: true
Attachment #711972 - Attachment is obsolete: true
Attachment #712004 - Flags: review?(sotaro.ikeda.g)
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
Attachment #712004 - Flags: review?(sotaro.ikeda.g) → review-
sotaro, absolutely!--if you have a more correct solution, feel free to propose a patch.
Created attachment 712539 [details] [diff] [review]
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.
Sotaro should there be a reviewer on this patch ?
(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
(In reply to dclarke@mozilla.com  [:onecyrenus] from comment #8)
> Sotaro should there be a reviewer on this patch ?

I am going to get reviewed by :kanru.
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.
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 on attachment 712539 [details] [diff] [review]
patch: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

kanru, can you review the patch? Thanks.
Attachment #712539 - Flags: review?(kchen)
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.
Attachment #712004 - Attachment is obsolete: true
kanru, do you have a time to review?
Created attachment 714432 [details] [diff] [review]
patch v2: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

fix -int(mSynchronousMode) adjustments. GonkNativeWindow always runs as if mSynchronousMode = true.
Attachment #712539 - Attachment is obsolete: true
Attachment #712539 - Flags: review?(kchen)

Updated

5 years ago
Attachment #714432 - Flags: review?(mhabicher)
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.
Attachment #714432 - Flags: review?(mhabicher) → review+
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.
Attachment #714432 - Attachment is obsolete: true
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 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?
Attachment #712539 - Attachment is obsolete: false
Attachment #712539 - Flags: review?(mhabicher)
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
Attachment #712539 - Flags: review?(mhabicher) → review+
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)
Created attachment 715602 [details] [diff] [review]
patch v3: add MIN_UNDEQUEUED_BUFFERS check in GonkNativeWindow::dequeueBuffer()

apply a comment. carry "kchen: review+"
Attachment #712539 - Attachment is obsolete: true
Attachment #715602 - Flags: review+

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/ceb97502eb31
status-b2g18-v1.0.0: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → fixed
Keywords: checkin-needed
Target Milestone: --- → B2G C4 (2jan on)
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
Attachment #715602 - Flags: approval-mozilla-b2g18?
tracking-b2g18: --- → ?
status-b2g18: --- → affected
status-b2g18-v1.0.1: --- → wontfix
tracking-b2g18: ? → +
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.
Attachment #715602 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Duplicate of this bug: 843616
https://hg.mozilla.org/mozilla-central/rev/ceb97502eb31
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
status-b2g18: affected → fixed
You need to log in before you can comment on or make changes to this bug.