Closed Bug 830995 Opened 11 years ago Closed 11 years ago

Camera - preview doesn't always restart after taking photos

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:-, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18+ fixed)

Status:
RESOLVED FIXED
Milestone:
B2G C4 (2jan on)
Project Flags:
blocking-b2g -
Tracking Flags:
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 + fixed

People

(Reporter: mikeh, Assigned: mikeh)

References

Details

Attachments

(6 files, 1 obsolete file)

A complete list of threads and where they're blocked
38.25 KB, text/plain
Details
wip patch: split thread
10.50 KB, patch
Details | Diff | Splinter Review
solution to the deadlock, v2 (landed)
15.69 KB, patch
kanru
: review+
sotaro
: feedback+
overholt
: approval-mozilla-b2g18+
Details | Diff | Splinter Review
log03_recurrence_with_fix_bt.txt
46.65 KB, text/plain
Details
log03_recurrence_with_fix_logcat.txt
322.11 KB, application/x-gzip
Details
logcat from following steps in comment 17
618.13 KB, text/plain
Details 
No clear STR yet; I have reproduced the issue twice: once after taking a half-dozen photos; and once after taking ~100.

Observed:
- the unagi viewfinder stays on the garbled postview screen
- the UI buttons appear to be enabled, but not active
- can switch back to the homescreen
- can switch back to the camera (though the preview does not resume)

From the logcat:

I( 1848:0x74f) 1833544[446472e0]: GonkNativeWindow::returnBuffer: slot=0 (generation=9)
I( 1848:0x74f) 1833544[446472e0]: virtual android::CameraGraphicBuffer::~CameraGraphicBuffer():259 : this=430220c0
I( 1848:0x738) 1074189560[42507160]: mozilla::StartPreviewTask::StartPreviewTask(mozilla::CameraControlImpl*, mozilla::DOMCameraPreview*):517 : this=430214a0
I( 1848:0x74a) 1829104[44646d30]: virtual nsresult mozilla::StartPreviewTask::Run():527
I( 1848:0x74a) 1829104[44646d30]: virtual nsresult mozilla::nsGonkCameraControl::StartPreviewImpl(mozilla::StartPreviewTask*): starting preview (mDOMPreview=44303ac0)
I( 1848:0x74a) 1829104[44646d30]: static int mozilla::GonkCameraHardware::StartPreview(uint32_t) : aHwHandle = 1, hw = 449d1b00
I( 1848:0x74a) 1829104[44646d30]: abandon: new generation 10
V( 1848:0x74a) enableMsgType(0)
V( 1848:0x74a) startPreview(0)
E( 1848:0x74a) Qint android::start_preview(camera_device*): E
V( 1848:0x74a) startPreview E
V( 1848:0x74a)  startPreview : Starting normal preview 
I( 1848:0x74a)  getBuffersAndStartPreview : E 
V( 1848:0x74a) getBuffersAndStartPreview: Calling native_window_set_buffer
I( 1848:0x74a) 1829104[44646d30]: setBufferCount: count=9
I( 1848:0x74a) 1829104[44646d30]: GonkNativeWindow::setBuffersDimensions
I( 1848:0x74a) 1829104[44646d30]: GonkNativeWindow::setBuffersFormat
I( 1848:0x74a) 1829104[44646d30]: GonkNativeWindow::setUsage
I( 1848:0x74a) 1829104[44646d30]: dequeueBuffer: E
D( 1759:0x6ec) /dev/pmem_adsp: Freeing buffer base:0x43d64000 size:233472 fd:101
D( 1759:0x6ec) /dev/pmem_adsp: Freeing buffer base:0x43eab000 size:233472 fd:104
D( 1759:0x6ec) /dev/pmem_adsp: Freeing buffer base:0x44600000 size:233472 fd:123
D( 1759:0x6ec) /dev/pmem_adsp: Freeing buffer base:0x44749000 size:233472 fd:127
D( 1759:0x6ec) /dev/pmem_adsp: Freeing buffer base:0x44b00000 size:233472 fd:140
D( 1759:0x6ec) /dev/pmem_adsp: Freeing buffer base:0x450e2000 size:233472 fd:143
D( 1759:0x6ec) /dev/pmem_adsp: Freeing buffer base:0x45129000 size:233472 fd:146
D( 1759:0x6ec) /dev/pmem_adsp: Allocated buffer base:0x43d64000 size:233472 fd:101
D( 1848:0x740) /dev/pmem_adsp: Mapped buffer base:0x42fa2000 size:233472, fd:34
I( 1848:0x740) 1817696[43e7a080]: GonkNativeWindow::returnBuffer: slot=7 (generation=9)
I( 1848:0x74a) 1829104[44646d30]: dequeueBuffer: returning slot=0 buf=431996f0 
I( 1848:0x74a) 1829104[44646d30]: dequeueBuffer: X
V( 1848:0x74a)  Locking buffer 0 
I( 1848:0x74a) 1829104[44646d30]: GonkNativeWindow::lockBuffer
E( 1848:0x74a)  Locked buffer 0 successfully
E( 1848:0x74a) Handle 0x431996f0, Fd passed:34, Base:0x42fa2000, Size 0x39000
V( 1848:0x74a) fd mmap fd 34 size 233472
E( 1848:0x74a)  Mapped Preview buffer 0
E( 1848:0x74a) Got the following from get_mem data: 0x438ea000, handle :1144595744, release : 0x40bd250d, size: 233472
E( 1848:0x74a)  getbuffersandrestartpreview deQ 34
E( 1848:0x74a) Registering buffer 0 with fd :34 with kernel
V( 1848:0x74a) use_all_chnls = 0
E( 1848:0x74a) register_buf: CbOff = 0x25800 CrOff = 0x0
V( 1848:0x74a) register_buf:  reg = 0 buffer = 0x438ea000
E( 1848:0x74a) Came back from register call to kernel
I( 1848:0x74a) 1829104[44646d30]: dequeueBuffer: E

...camera driver thread is now frozen/deadlocked.
sotaro, I'm still looking into where the last call to dequeueBuffer() (above) finally stops, but could this be related to the work you've been doing in bug 826829 and bug 827833 ?
Assignee: nobody → mhabicher
blocking-b2g: --- → tef?
(In reply to Mike Habicher [:mikeh] from comment #1)
> sotaro, I'm still looking into where the last call to dequeueBuffer()
> (above) finally stops, but could this be related to the work you've been
> doing in bug 826829 and bug 827833 ?

Almost case is related to bug 826829, I think.
The camera driver thread deadlocks in GonkNativeWindow::dequeueBuffer(); this function call never returns:

        ibc->AllocSurfaceDescriptorGralloc(gfxIntSize(mDefaultWidth, mDefaultHeight),
                                           mPixelFormat,
                                           mUsage,
                                           &buffer);
sotaro, bad news--my build includes your changes from bug 826829.

Other bad news: once in this condition, gdb won't attach to the camera process, so I can't see where the various threads are parked.
Wait, I got lucky and gdb attached!  Here's the backtrace of the camera driver thread:

(gdb) thread 14
[Switching to thread 14 (Thread 2974.2994)]#0  __futex_syscall3 () at bionic/libc/arch-arm/bionic/atomics_arm.S:182
182	    swi     #0
(gdb) bt
#0  __futex_syscall3 () at bionic/libc/arch-arm/bionic/atomics_arm.S:182
#1  0x400db55c in __pthread_cond_timedwait_relative (cond=0x46eb23d4, mutex=0x45ef2060, reltime=0x0) at bionic/libc/bionic/pthread.c:1477
#2  0x400db610 in __pthread_cond_timedwait (cond=0x46eb23d4, mutex=0x45ef2060, abstime=0x0, clock=0) at bionic/libc/bionic/pthread.c:1500
#3  0x400510c6 in PR_WaitCondVar (cvar=0x46eb23d0, timeout=4294967295) at /home/mikeh/dev/mozilla/m-c/inbound-src/nsprpub/pr/src/pthreads/ptsynch.c:385
#4  0x412aab52 in mozilla::CondVar::Wait (this=0x454d3c54, interval=4294967295) at /home/mikeh/dev/mozilla/btg019/objdir-gecko-inbound-debug/xpcom/build/BlockingResourceBase.cpp:340
#5  0x413a0b38 in mozilla::Monitor::Wait (this=<value optimized out>, aSize=<value optimized out>, aFormat=@0x466cae88, aUsage=@0x466cae8c, aBuffer=0x454d3cbc) at ../../dist/include/mozilla/Monitor.h:47
#6  mozilla::layers::ImageBridgeChild::AllocSurfaceDescriptorGralloc (this=<value optimized out>, aSize=<value optimized out>, aFormat=@0x466cae88, aUsage=@0x466cae8c, aBuffer=0x454d3cbc)
    at /home/mikeh/dev/mozilla/m-c/inbound-src/gfx/layers/ipc/ImageBridgeChild.cpp:351
#7  0x40c7d938 in android::GonkNativeWindow::dequeueBuffer (this=0x466ca800, buffer=<value optimized out>) at /home/mikeh/dev/mozilla/m-c/inbound-src/dom/camera/GonkNativeWindow.cpp:266
#8  0x40c7cece in android::GonkNativeWindow::hook_dequeueBuffer (window=0x46eb23d4, buffer=0x80) at /home/mikeh/dev/mozilla/m-c/inbound-src/dom/camera/GonkNativeWindow.cpp:97
#9  0x40c7b710 in android::CameraHardwareInterface::__dequeue_buffer (w=<value optimized out>, buffer=0x454d3d7c, stride=0x0)
    at /home/mikeh/dev/mozilla/btg019/frameworks/base/services/camera/libcameraservice/CameraHardwareInterface.h:582
#10 0x42faec26 in android::QualcommCameraHardware::getBuffersAndStartPreview() () from /home/mikeh/dev/mozilla/btg019/out/target/product/unagi/system/lib/hw/camera.msm7627a.so
#11 0x42faec26 in android::QualcommCameraHardware::getBuffersAndStartPreview() () from /home/mikeh/dev/mozilla/btg019/out/target/product/unagi/system/lib/hw/camera.msm7627a.so
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
AllocSurfaceDescriptorGralloc() is out side the scope of bug 826829.
bug 826829, only care about conflict between GonkNativeWindwo::abandon() and ImageContainerChild::SetIdleNow(). Therefore it is necessary to fix deadlock problem.

split threads between ImageBridgeChild and ImageContainerChild is the way to go, I think.
Because the above frame is wedged, the mMutex that is locked at the beginning GonkNativeWindow::dequeueBuffer() is never unlocked, and the camera preview thread is also stuck:

(gdb) thread 9
[Switching to thread 9 (Thread 2974.2984)]#0  __futex_syscall3 () at bionic/libc/arch-arm/bionic/atomics_arm.S:182
182	    swi     #0
(gdb) bt
#0  __futex_syscall3 () at bionic/libc/arch-arm/bionic/atomics_arm.S:182
#1  0x400db254 in _normal_lock (mutex=0x466cae94) at bionic/libc/bionic/pthread.c:951
#2  pthread_mutex_lock (mutex=0x466cae94) at bionic/libc/bionic/pthread.c:1041
#3  0x40c7d3e8 in android::Mutex::lock (this=0x466ca800, aIndex=2, aGeneration=20) at /home/mikeh/dev/mozilla/btg019/frameworks/base/include/utils/threads.h:289
#4  Autolock (this=0x466ca800, aIndex=2, aGeneration=20) at /home/mikeh/dev/mozilla/btg019/frameworks/base/include/utils/threads.h:244
#5  android::GonkNativeWindow::returnBuffer (this=0x466ca800, aIndex=2, aGeneration=20) at /home/mikeh/dev/mozilla/m-c/inbound-src/dom/camera/GonkNativeWindow.cpp:400
#6  0x40c7d494 in android::CameraGraphicBuffer::Unlock (this=0x444cbe00) at /home/mikeh/dev/mozilla/m-c/inbound-src/dom/camera/GonkNativeWindow.h:270
#7  0x413a85d0 in ~GonkIOSurfaceImage (this=0x444cbe40, __in_chrg=<value optimized out>) at /home/mikeh/dev/mozilla/m-c/inbound-src/gfx/layers/GonkIOSurfaceImage.h:64
#8  0x413a8626 in ~GonkIOSurfaceImage (this=0x466cae94, __in_chrg=<value optimized out>) at /home/mikeh/dev/mozilla/m-c/inbound-src/gfx/layers/GonkIOSurfaceImage.h:65
#9  0x40cb02e0 in mozilla::layers::Image::Release (this=0x444cbe40) at ../../dist/include/ImageContainer.h:67
#10 0x413a27de in ~nsRefPtr (this=0x448ed6ec, count=1, start=<value optimized out>) at ../../dist/include/nsAutoPtr.h:876
#11 nsTArrayElementTraits<nsRefPtr<mozilla::layers::Image> >::Destruct (this=0x448ed6ec, count=1, start=<value optimized out>) at ../../dist/include/nsTArray.h:432
#12 nsTArray_Impl<nsRefPtr<mozilla::layers::Image>, nsTArrayInfallibleAllocator>::DestructRange (this=0x448ed6ec, count=1, start=<value optimized out>) at ../../dist/include/nsTArray.h:1297
#13 nsTArray_Impl<nsRefPtr<mozilla::layers::Image>, nsTArrayInfallibleAllocator>::RemoveElementsAt (this=0x448ed6ec, count=1, start=<value optimized out>) at ../../dist/include/nsTArray.h:1017
#14 0x413a2844 in nsTArray_Impl<nsRefPtr<mozilla::layers::Image>, nsTArrayInfallibleAllocator>::RemoveElementAt (this=0x448ed6c0, aImage=...) at ../../dist/include/nsTArray.h:1023
#15 mozilla::layers::ImageContainerChild::RecvReturnImage (this=0x448ed6c0, aImage=...) at /home/mikeh/dev/mozilla/m-c/inbound-src/gfx/layers/ipc/ImageContainerChild.cpp:131
#16 0x4116ace0 in mozilla::layers::PImageContainerChild::OnMessageReceived (this=0x448ed6c0, __msg=...) at /home/mikeh/dev/mozilla/btg019/objdir-gecko-inbound-debug/ipc/ipdl/PImageContainerChild.cpp:401
#17 0x41168990 in mozilla::layers::PImageBridgeChild::OnMessageReceived (this=0x43d28540, __msg=...) at /home/mikeh/dev/mozilla/btg019/objdir-gecko-inbound-debug/ipc/ipdl/PImageBridgeChild.cpp:486
#18 0x410916f2 in mozilla::ipc::AsyncChannel::OnDispatchMessage (this=0x43d28548, msg=...) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/glue/AsyncChannel.cpp:473
#19 0x410980bc in mozilla::ipc::RPCChannel::OnMaybeDequeueOne (this=0x43d28548) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/glue/RPCChannel.cpp:402
#20 0x41067754 in DispatchToMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)()> (this=<value optimized out>) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/tuple.h:383
#21 RunnableMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)(), Tuple0>::Run (this=<value optimized out>) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/task.h:307
#22 0x4109650e in mozilla::ipc::RPCChannel::RefCountedTask::Run (this=0x444eda60) at ../../dist/include/mozilla/ipc/RPCChannel.h:425
#23 mozilla::ipc::RPCChannel::DequeueTask::Run (this=0x444eda60) at ../../dist/include/mozilla/ipc/RPCChannel.h:448
#24 0x41317a0a in MessageLoop::RunTask (this=0x44179dd0, task=0x444eda60) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/message_loop.cc:333
#25 0x41318234 in MessageLoop::DeferOrRunPendingTask (this=0x466cae94, pending_task=<value optimized out>) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/message_loop.cc:341
#26 0x41318f86 in MessageLoop::DoWork (this=0x44179dd0) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/message_loop.cc:441
#27 0x41319302 in base::MessagePumpDefault::Run (this=0x4339af00, delegate=0x44179dd0) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/message_pump_default.cc:23
#28 0x41317fbe in MessageLoop::RunInternal (this=0x44179dd0) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/message_loop.cc:215
#29 0x4131801e in MessageLoop::RunHandler (this=0x44179dd0) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/message_loop.cc:208
#30 MessageLoop::Run (this=0x44179dd0) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/message_loop.cc:182
#31 0x41321f84 in base::Thread::ThreadMain (this=0x43d15b80) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/thread.cc:156
#32 0x4132fb4a in ThreadFunc (closure=0x466cae94) at /home/mikeh/dev/mozilla/m-c/inbound-src/ipc/chromium/src/base/platform_thread_posix.cc:39
#33 0x400dbe18 in __thread_entry (func=0x4132fb41 <ThreadFunc>, arg=0x43d15b80, tls=<value optimized out>) at bionic/libc/bionic/pthread.c:217
#34 0x400db96c in pthread_create (thread_out=<value optimized out>, attr=0xbec9f3e8, start_routine=0x4132fb41 <ThreadFunc>, arg=0x43d15b80) at bionic/libc/bionic/pthread.c:357
#35 0x00000000 in ?? ()

(Included here for completeness--it is most likely a symptom of a problem rather than the cause.)
This keeps the mSlots[] mMutex separate from the Gralloc operation in dequeueBuffer(), so that buffers can be returned without a deadlock.

I haven't tested this or even tried to compile it--just want feedback on this approach.
Attachment #702629 - Flags: feedback?(sotaro.ikeda.g)
Comment on attachment 702629 [details] [diff] [review]
WIP - a potential solution to the deadlock

There is also ImageBridgeChild::DeallocSurfaceDescriptorGralloc(). It is also necessary need to care about. The function is called in GonkNativeWindow::freeBufferLocked(int i).

Mutex is held to keep slots states to consistent. If we unlock the mutex, this consistency could be broken. Other client could call GonkNativeWindow's function and update slots states. This could introduce timing related bugs.
Attachment #702629 - Flags: feedback?(sotaro.ikeda.g) → feedback-
Split ImageBridgeChild thread and ImageContainerChild thread.

But it seems that ipc do not work correctly when Camera app is run in chrome process. In one case, ImageContanerChild::SendFlush() do not complete, even when ImageContanerParent processed it.
Attachment #702798 - Attachment is patch: true
Take a look at this one.  This includes the previous changes (which could probably stand to be streamlined) but also breaks up removing SurfaceDesciptors from mSlots with mMutex locked, and actually calling Dealloc..Gralloc() on them (with mMutex unlocked).

This is tested--I took about 100 photos--and didn't encounter any problems.
Attachment #702629 - Attachment is obsolete: true
Attachment #702869 - Flags: feedback?(sotaro.ikeda.g)
Comment on attachment 702869 [details] [diff] [review]
solution to the deadlock, v2 (landed)

It sounds good to me. It is very good jod!
I thought too much not to touch GonkNativeWindow.
Attachment #702869 - Flags: feedback?(sotaro.ikeda.g) → feedback+
blocking-b2g: tef? → -
tracking-b2g18: --- → +
With the above 'WIP' patch, the preview still occasionally fails to restart, but the failure mode is significantly different.  In this case it looks like the runSnapshotThread() function in the driver is blocked, waiting for a signal on mJpegThreadWait (that never comes).  See thread 24 in this log--it will correlate with the logcat output I will attach next.
STR! (with the patch from comment 13 applied):
1. open camera app
2. switch to video mode
3. press start recording and record a ~10s video
4. press stop recording
5. press start recording and record a ~10s video
6. press stop recording
7. switch to picture mode
8. press the shutter button

Preview is now frozen; the UI is still responsive, however, and force-closing the camera app will clear the problem.

As with the log attached to comment 16, the camera driver is waiting for a signal on mJpegThreadWait: "V( 1288:0x54a) runSnapshotThread: waiting for jpeg callback."
After "waiting for jpeg callback", logcat shows this:

V( 1526:0x65f) runSnapshotThread: waiting for jpeg callback.
E( 1526:0x660) PROFILE snap got the image: 1358375849.394169394
E( 1526:0x660) PROFILE snap config encoding: 1358375849.394260946
E( 1526:0x660) PROFILE snap starting encoding: 1358375849.394413534
E( 1526:0x660) PROFILE encoder configure: 1358375849.394566122
E( 1526:0x660) PROFILE HW encoder starting encoding: 1358375849.395268026

...then nothing.
(Also: the steps in comment 17 don't have to be done particularly quickly.)
Bingo?

logcat output is difference after recording the first video (comment 17, steps 3 and 4) and the second (steps 5 and 6).  After the latter, the log shows:

E( 1712:0x6b0) [JavaScript Error: "NS_ERROR_NOT_AVAILABLE: Component is not available" {file: "app://camera.gaiamobile.org/js/filmstrip.js" line: 489}]

...and the rest of the video recording "wrap-up" output doesn't appear (presumably it is not done?).
The (apparently) offending line:

    // Draw that region of the image into the canvas, scaling it down
    context.drawImage(elt, x, y, w, h,
                      0, 0, THUMBNAIL_WIDTH, THUMBNAIL_HEIGHT);
Compare lines 2836 and 5083.

~2836:
E( 1712:0x6dc) ***YAMATO Enabled***
E( 1712:0x6d8) Flush VDL_stats_q: stats_ptr 0x43e404a0
E( 1712:0x6d8) Warning - previous ts > current ts. And both are non B-frames

~5083:
E( 1712:0x6ec) ***YAMATO Enabled***
E( 1712:0x6b0) [JavaScript Error: "NS_ERROR_NOT_AVAILABLE: Component is not available" {file: "app://camera.gaiamobile.org/js/filmstrip.js" line: 489}]
djf, any idea what the JavaScript error in comment 22 means?
Flags: needinfo?(dflanagan)
daleharvey too, any idea what the JavaScript error in comment 22 means?
Flags: needinfo?(dale)
Comment on attachment 702869 [details] [diff] [review]
solution to the deadlock, v2 (landed)

kanru, would you mind reviewing this?  It fixes the camera lock-up problem while taking pictures; everything else seems to be a different issue.
Attachment #702869 - Flags: review?(kchen)
Mike,

I haven't see that error before. If the line number is right, its occuring when we try to copy an image into a <canvas> element.  So an error from graphics somehow? I know nothing about that... out of memory? Would an image copy like that get accelerated in the graphics hardware? 

I seem to be able to reproduce this bug by launching video, playing and pausing a video and then going to the camera.  First time I take a picture, it freezes.
Flags: needinfo?(dflanagan)
Mike,

Also note that that call to createThumbnailFromElement() is coming from line 331.  When we get a new jpeg file from the camera, we parse its metadata, use blob.slice() to get the preview image out of it, and then create a smaller square thumbnail from that rectangular preview.

Is it possible that the camera is giving us the jpeg image before it is done writing the preview image metadata?  Maybe there is a timing issue here and if videos have been playing or recording, something is slower and we try to use the preview image before it is written to the file?  Its a complete guess, of course, but I think we had a similar issue with the video... we finished recording, but the rotation metadata wasn't fully written into the video file yet.
(In reply to David Flanagan [:djf] from comment #26)
> 
> I seem to be able to reproduce this bug by launching video, playing and
> pausing a video and then going to the camera.  First time I take a picture,
> it freezes.

djf, that's a great data point!  I bet there's a shared resource that does both video playback and the JPEG encoding, and I'm guessing the video playback subsystem isn't releasing it.

(In reply to David Flanagan [:djf] from comment #27)
> 
> Is it possible that the camera is giving us the jpeg image before it is done
> writing the preview image metadata?  Maybe there is a timing issue here and
> if videos have been playing or recording, something is slower and we try to
> use the preview image before it is written to the file?  Its a complete
> guess, of course, but I think we had a similar issue with the video... we
> finished recording, but the rotation metadata wasn't fully written into the
> video file yet.

djf, in this case the JavaScript exception happens while trying to generate the thumbnail for a movie.  I wonder if the exception is causing the media extractor to not get cleaned up, leading to the shared resource conflict above.

I realize this is all a little handwavy, but it's all I've got to go on right now.
(In reply to David Flanagan [:djf] from comment #26)
> 
> I seem to be able to reproduce this bug by launching video, playing and
> pausing a video and then going to the camera.  First time I take a picture,
> it freezes.

I am going to think it might be a system resources problem. Chipset of FirefoxOS phone do not have a lot of system resources. Therefore when take a picture by camera app, all OMXCodecs need to be freed. This is just a possibility.
David cleared as much as needinfo as I could, gaia isnt doing anything weird here so seems like it must be related to system media resources
Flags: needinfo?(dale)
Attachment #702869 - Flags: review?(kchen) → review+
(In reply to Mike Habicher [:mikeh] from comment #28)
> (In reply to David Flanagan [:djf] from comment #26)
> djf, that's a great data point!  I bet there's a shared resource that does
> both video playback and the JPEG encoding, and I'm guessing the video
> playback subsystem isn't releasing it.

> djf, in this case the JavaScript exception happens while trying to generate
> the thumbnail for a movie.  I wonder if the exception is causing the media
> extractor to not get cleaned up, leading to the shared resource conflict
> above.

To solve the problem, apps need to free OMXCodec and camera resource. Off the top of my head, threre are two ways to do this.
[1] application frees hw codec related resources, when it goes to background.
[2] gecko frees hw codec related resources, when it goes to background.

[2] is necessary as a long term solution. I am not sure, FirefoXOS v1 could solves the problem only by [1].
> To solve the problem, apps need to free OMXCodec and camera resource. Off
> the top of my head, threre are two ways to do this.
> [1] application frees hw codec related resources, when it goes to background.
> [2] gecko frees hw codec related resources, when it goes to background.
> 
> [2] is necessary as a long term solution. I am not sure, FirefoXOS v1 could
> solves the problem only by [1].

I created Bug 831747 for [2].
Attachment #702869 - Attachment description: WIP - a potential solution to the deadlock, v2 → solution to the deadlock, v2 (landed)
Comment on attachment 702869 [details] [diff] [review]
solution to the deadlock, v2 (landed)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): potential deadlock between ImageBridge and camera driver threads
User impact if declined: camera driver and ImageBridge will occasionally deadlock (sometimes after 1 photo, sometimes after 100); once this happens, device must be restarted to unstick camera
Testing completed: several hundred photos taken
Risk to taking this patch (and alternatives if risky): low, fix is limited to GonkNativeWindow internal functions, and simply delays calls to Gralloc() functions until the internal mMutex is unlocked.
String or UUID changes made by this patch: none
Attachment #702869 - Flags: approval-mozilla-b2g18?
Attachment #702869 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/e6bbe30c5987
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: qawanted
Resolution: --- → FIXED
(Migrated most of the comments on the issue identified in comment 17 to bug 831747.)
(In reply to Sotaro Ikeda [:sotaro] from comment #31)

> To solve the problem, apps need to free OMXCodec and camera resource. Off
> the top of my head, threre are two ways to do this.
> [1] application frees hw codec related resources, when it goes to background.
> [2] gecko frees hw codec related resources, when it goes to background.

Is this still necessary even with the patch that Mike has just landed?

If so, do we need to investigate option 1 and try fixing this in gaia?

What does it mean to free hw codec related issues? Would the video app have to destroy its <video> element when it is hidden and then recreate it to resume playback at the same position when it becomes visible again?  That might be a little tricky, but probably doable.

If it is just a matter of properly freeing the <video> tag used to create thumbnails of recently recorded videos, that is easy and we should fix it for the 1.0.0.0 release.

I don't understand this bug well enough to know what is needed, but please keep Dale and I informed about what Gaia changes are needed. Maybe file another bug and assign it to me for the required Gaia changes?
(In reply to Mike Habicher [:mikeh] from comment #28)
> djf, in this case the JavaScript exception happens while trying to generate
> the thumbnail for a movie.  I wonder if the exception is causing the media
> extractor to not get cleaned up, leading to the shared resource conflict
> above.
> 
> I realize this is all a little handwavy, but it's all I've got to go on
> right now.

I see what you're saying... The exception in the canvas does prevent the video element form being cleaned up.  We can fix that with try/catch if we need to.  It doesn't explain why we're getting that exception in the first place, though.  

Does this need to be addressed in bug 831747, or does the gecko patch fix it?
(In reply to David Flanagan [:djf] from comment #38)
> 
> Does this need to be addressed in bug 831747, or does the gecko patch fix it?

djf and I have discussed this in IRC already; just closing off this thread for archaeologists:
- the gecko patch attached to _this_ bug does not address the above (new) issue
- further work on this issue will be handled in bug 831747 or other bugs.
status-b2g18: --- → fixed
status-firefox19: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → fixed
Target Milestone: --- → B2G C4 (2jan on)
Build id:20130208070201

This issue is not reproducing.
According to this bug, after recording the vedio for couple of times and going back to camera and taking pictures, the preview is frozen and force-closing the camera app will clear the problem.

But the preview is not frozen and the camera is working fine.
Depends on: 852766
No longer depends on: 852766
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: