975597 - [B2G][Camera] Connecting to a PC with USB storage enabled while recording a video will cause the media apps to be disabled

Status: RESOLVED FIXED

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

Description

[Salem Elkabule]

Description:
While recording a video with the camera and connecting the USB cable from the computer and unplugging it, the message will pop up “Camera cannot be used while plugged in”. The message cannot be dismissed until rebooting the device. This issue effect all the media apps "Gallery, camera, music, Videos"


Repro Steps:
1) Updated Buri to Build ID: 20140220040203
2) Proceed to settings and enable USB storage
3) Open the Camera app and start recording a video 
4) Plug the USB cable into the Buri while recording
5) Unplug the USB cable from the Buri
6) Close and launch any of the media apps and observe


Actual:
The USB connected screen “Camera cannot be used while plugged in” cannot be dismissed.
 
Expected:
The USB connected screen must dismiss when unplugging the device.

Environmental Variables
Device: buri 1.4 MOZ RIL
Build ID: 20140220040203
Gecko: https://hg.mozilla.org/mozilla-central/rev/660b62608951
Gaia: 6e71ab4da1b08586ea0c758edb7aa199ee34cd2f
Platform Version: 30.0a1
Firmware Version: V1.2-device.cfg

Repro frequency: 100%
See attached: Video (http://www.youtube.com/watch?edit=vd&v=3hVJsllAA9A)

Comment 1

[Salem Elkabule]

This issue does not reproduce on 1.3

Environmental Variables
Device: buri 1.3 MOZ RIL
Build ID: 20140221004002
Gecko: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/e5f25becc0e7
Gaia: 8039a5cb7519adfa81677df577f494c6a4de6140
Platform Version: 28.0
Firmware Version: V1.2-device.cfg

The USB connected screen does get dismissed

Comment 2

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

Regression from bug 958200?

Comment 3

[Matthew Vaughan]

This issue started reproducing on the first available 1.4 build.

Device: Buri v1.4 MOZ RIL
BuildID: 20131210040206
Gaia: c952e2756c03eceb4de6a3eba15651741a62f9e8
Gecko: df82be9d89a5
Version: 29.0a1
Firmware Version: V1.2-device.cfg

Comment 4

[Hema Koka [:hema]]

Diego - I know you are in the workshop, but please provide your input so we can have Punam help fix this regression issue.

Thanks
Hema

Comment 5

[Diego Marcos [:dmarcos]]

The media apps seem to behave as expected. With USB storage enabled the system locks and releases the storage when plugging and unplugging the cable. You can see that in settings/media storage. When plugging the cable while recording a video the system takes a hold of the storage and never releases it. In settings/media storage you can see all the SD Card Storage marked as not available. Unplugging the cable or enabling/disabling USB storage don't have any effect. Only restarting the device gets the USB storage to its normal state.

Comment 6

[Jason Smith [:jsmith]]

(In reply to Diego Marcos from comment #5)
> The media apps seem to behave as expected. With USB storage enabled the
> system locks and releases the storage when plugging and unplugging the
> cable. You can see that in settings/media storage. When plugging the cable
> while recording a video the system takes a hold of the storage and never
> releases it. In settings/media storage you can see all the SD Card Storage
> marked as not available. Unplugging the cable or enabling/disabling USB
> storage don't have any effect. Only restarting the device gets the USB
> storage to its normal state.

So are you implying this is a gecko bug?

Comment 7

[Dave Hylands [:dhylands]]

What should happen with the above STR is this:

1 - You start recording a video. This opens a file on the sdcard and holds it open
2 - You enable UMS and plug in the USB cable. The AutoMounter should notice that there are files open and defer the sharing until all files are closed. It will keep rechecking every 5 seconds to see if there are still files open.
3 - Unplugging the USB cable, things should go back to the state they were in from step 1.

Having said that, I don't recall ever testing this particular scenario, so its possible that there is some type of gecko bug. Its also possible that there is a gaia bug, since the mediadb.js code is the interface to device storage.

It sounds like some state information isn't being reflected properly. I'll see if I can reproduce.

Comment 8

[David Flanagan [:djf]]

The original reports says that this affects all the apps. If so, then there is nothing specific to recording videos here.

Salem: is this a camera-specific bug, or do you have the same issue will all media apps after plugging and unplugging the usb cable?  Or are you saying that you only have this issue in the other media apps after doing this usb cable thing while recording a video?

Dave: if all of the apps are getting wedged like this, then it isn't a state issue in the app itself or in mediadb and must be system wide.  Are you willing to assign this to yourself while you investigate?

Comment 9

[Salem Elkabule]

(In reply to David Flanagan [:djf] from comment #8)
> The original reports says that this affects all the apps. If so, then there
> is nothing specific to recording videos here.
> 
> Salem: is this a camera-specific bug, or do you have the same issue will all
> media apps after plugging and unplugging the usb cable?  Or are you saying
> that you only have this issue in the other media apps after doing this usb
> cable thing while recording a video?

Yes David I only get this issue in other media apps after plugging the USB cable while recording.

Comment 10

[Dave Hylands [:dhylands]]

Assigning to me while investigating whats going on here.

Comment 11

[Dave Hylands [:dhylands]]

I was able to reproduce on my unagi.

When I plugged in the USB cable I observed:

UpdateState: umsAvail:1 umsEnabled:1 mode:1 usbCablePluggedIn:1 tryToShare:1
UpdateState: Volume sdcard is Mounted and inserted @ /mnt/sdcard gen 1 locked 0 sharing y
Volume sdcard: IsSharing set to 1 state Mounted
The following files are open under '/mnt/sdcard'
  PID: 110 file: '/mnt/sdcard/1393452837497_tmp.3gp' app: '' comm: 'b2g' exe: '/system/b2g/b2g'
  PID: 509 file: '/mnt/sdcard/1393452837497_tmp.3gp' app: 'Camera' comm: 'Camera' exe: '/system/b2g/plugin-container'
  PID: 509 file: '/mnt/sdcard/1393452837497_tmp.3gp' app: 'Camera' comm: 'Camera' exe: '/system/b2g/plugin-container'
  PID: 509 file: '/mnt/sdcard/1393452837497_tmp.3gp' app: 'Camera' comm: 'Camera' exe: '/system/b2g/plugin-container'
UpdateState: Mounted volume sdcard has open files, not sharing or formatting
Changing status from 'Enabled' to 'FilesOpen'

And then it repeats the open files messages every 5 seconds.

So as far as I can tell everything is working properly from the gecko side. The act of plugging in the USB cable initiated the sharing operation (which marks the volume as unavailable).

What I think is supposed to happen is that the camera is supposed to stop recording, which will close the files and make the volume finish being shared with the PC.

I killed the camera app (long press on the home key and swipe up), and the main process is continuing to hold a file open:

PID: 110 file: '/mnt/sdcard/1393452837497_tmp.3gp (deleted)' app: '' comm: 'b2g' exe: '/system/b2g/b2g'

I think that this may be related to bug 910498, where the video file is opened by the parent, and then the open file handle is passed to the child. I think that the parent should close the file handle as soon as it passes it to the child.

A similar thing happens, if you're playing music, and plug the phone in, except that the Music app stops, and since there aren't any files open after that it shares the sdcard with the PC.

So I think that there are 2 bugs here:

1 - a camera bug where it doesn't stop recording when told that the sdcard became unavailable
2 - a gecko bug, where it's holding a file open that it shouldn't.

Comment 12

[Dave Hylands [:dhylands]]

ok - here's what I've discovered.

Once the patches for bug 977372 and bug 977373 are applied, then the flow goes something like the following:

1 - User starts recording a video
2 - User presses STOP
3 - This kicks off some background stuff to create the poster and store the temporary video file as a properly named DCIM file. The onStored method in camera/js/lib/storage.js calls storage.get() which creates a blob and subsequent of that blob causes a file descriptor to be opened.
4 - The blob for the temporary file gets deleted, but the blob stays around, and hence the file descriptor stays open.

I've tracked it as far as the showVideo function in camera/js/views/confirm.js, but I'm lost after this.

As soon as the home button is pushed after this point in time, then I think that the video blob gets garbage collected (or something) and that causes the file descriptor to be closed.

As long as the file descriptor is open, the USB Mass Storage is unable to share the volume with the PC.

You can see that the file descriptor is still open, by doing the following (on the host):

adb shell lsof | grep sdcard

You'll get something like the following out:

plugin-co   439    app_439   53       ???                ???       ???        ??? /mnt/sdcard/1394072828772_tmp.3gp

after the file is deleted, then it changes slightly and looks like:

plugin-co   439    app_439   53       ???                ???       ???        ??? /mnt/sdcard/1394072828772_tmp.3gp (deleted)

So I'm going to change the owner back to diego since this now looks like a js problem. I'll also go ahead and land bug 977372 (bug 977373 has already landed).

Comment 13

[Dave Hylands [:dhylands]]

I thought I would add, that for even a small 2-3 second video, it can take 30-45 seconds from the time I hit stop, until I see the preview show up at the top of the preview window.

Comment 14

[Hema Koka [:hema]]

If there is anyone who can help with this, please feel free to take it up

Comment 15

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Dave: Here's a patch to resolve this. Basically, it stops recording immediately if the storage `statechange` becomes "shared" (plugged in). Previously, I believe the app was continuing to attempt to record while the USB was plugged in and there was no way to gracefully stop the recording which is why it seemed to be holding on to the file.

Comment 16

[Dave Hylands [:dhylands]]

So this patch doesn't seem to address the issue.

STR:

- Make sure USB Mass Storage is OFF
- Launch Camera app
- Start recording a video
- Stop recording a video
- Wait for preview thumbnail to show up (seems to take upto about a minute)

From a terminal window, running:

adb shell lsof | grep sdcard

yields:

plugin-co   507    app_507   34       ???                ???       ???        ??? /mnt/sdcard/DCIM/112MZLLA/VID_0004.3gp

So even though its finished with the video, its still holding the file open. If I wait for about another minute, then the file does eventually close, so maybe this is a non-problem?

When I tried the original STR, then the camera app crashed (on my unagi) when I plugged in the USB cable. I'm trying to investigate that a bit further.

Comment 17

[Dave Hylands [:dhylands]]

On my unagi, with your patch applied, the camera app seems to crash (2 out of 2 times) and didn't crash (2 out of 2 times) without your patch applied.

I'll try on my hamachi to see if it behaves the same.

Comment 18

[Dave Hylands [:dhylands]]

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

Tested on hamachi debug and non-debug builds and it seems to work as advertised.

I'm not sure what the problem on the unagi is, and since its not a supported phone any more, it really doesn't matter.

So giving this an r+

I made one comment on the PR about testing != "available" rather than testing == "shared"

Comment 19

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/5e9c063b348ccabc88aa4b4a1a6dea79b01deef1

Comment 20

[Dave Hylands [:dhylands]]

The only thing I can think of for keeping a file open is to create a file, and then do a get (which returns a Blob) and then read a slice from the Blob.

You should be able to test this using:

adb shell lsof | grep sdcard

to see what files are open on the sdcard.

Comment 21

[Justin D'Arcangelo [:justindarc]]

Thanks Dave! I will try that out. That would probably be a more robust solution and may help with the Unagi problem.

Comment 22

[David Flanagan [:djf]]

Bulk edit for camera bugs.

If earlier comments do not show how this bug landed to master, it probably landed as part of https://github.com/mozilla-b2g/gaia/pull/17599 which merged the camera-new-features branch into master.

This bug was uplifted from master to v1.4 as part of https://github.com/mozilla-b2g/gaia/commit/a8190d08e61316a86bba572ba8d894d081a20530