1206646 - [Aries KK]Connect device with computer by USB(UMS), then record a video, tap the Stop button, it does not react.

Status: RESOLVED WONTFIX

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

Description

[Autumn.Qiu(Leave from Mozilla)]

Attachment: logcat_video.txt

[1.Description]:
[Aries KK v2.5][Camera]Connect device with computer, launch Setting -> USB Storage, then choose UMS and open USB Storage.Tap Home button and Launch Camera, switch to VIDEO mode.Record a video, and wait a second, tap Stop button. the Stop button does not react.
Found at: 01:27
See attachment:logcat_video.txt & Flame KK v2.5_video.3gp.

[2.Testing Steps]: 
Premise: there is a SD card in device and default media location is internal.

1.Connect device with compute.
2.Launch Setting -> USB Storage.
3.Choose UMS and open USB Storage.
4.Tap home button and Launch Camera.
5.Switch to VIDEO mode.
6.Record a video.
7.Wait a second, tap Stop button.

[3.Expected Result]: 
In step 7, device will stop record.

[4.Actual Result]: 
In step 7, the Stop button does not react and recording still continues.

[5.Reproduction build]: 
Device: Aries KK 2.5  (Affected) 
Build ID               20150920050928
Gaia Revision          e67d319d0854e32e23210784eb9c4e1b8a025adb
Gaia Date              2015-09-19 07:42:05
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/ccd6b5f5e544c1d708849144943a776941bd3794
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150920.042926
Firmware Date          Sun Sep 20 04:29:33 UTC 2015
Bootloader             s1


[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

[8.NOTE]
If the same operation is performed on the Flame KK v2.5 , in step 4 ,there will be a warning "Unplug  the phone to capture photos and videos" and Camera can't launch.

Comment 1

[Autumn.Qiu(Leave from Mozilla)]

Attachment: Flame KK v2.5_video.3gp

Comment 2

[Norry.L.F(Leave from Mozilla)]

Hi No-jun,

Could you help to dispatch this bug?  thanks :)

Comment 3

[No-Jun Park [:njpark]]

Hmm, I thought in UMS mode, it should ask the use to disconnect before using camera.  Pinging dhyland, wondering whether this is a valid scenario.

Comment 4

[Dave Hylands [:dhylands]]

I don't know what's valid from the UX perspective.

From a technical perspective, UMS mode only affects the external sdcard.

On the aries phone, internal storage is always available.
On the flame, internal storage is NOT available when using UMS mode.

It also depends on what the default setting for the storage location is (internal or external).

I can see 2 scenarios:

1 - Default location is internal. We could allow the recording to work normally.

2 - Default location is external. We can't record to external when UMS is enabled and the external volume is being shared.

I believe that the mediadb is setup such if any volume is being shared (via UMS), then we consider all of them to unavailable.

Comment 5

[No-Jun Park [:njpark]]

ni?ing :tif for answers to Comment 4

Comment 6

[Tiffanie Shakespeare [:tif]]

As long the recording can be saved then we should let the user take videos. If the start button works, then the user needs to be able to stop the recording.

If the recording is unable to be saved, e.g. the Flame, then we should prevent the user from starting a recording.

Let me know if there are more questions!

Comment 7

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:

Per comment 6, unable to stop recording that was started under the same condition seems pretty risky to me, as someone might want to record while the phone is connected to pc.  Nominating to 2.5 to start the discussion.

Comment 8

[Hema Koka [:hema]]

What happens when you unplug USB or press home button. Does the video stop recording then and save it? Adding qawanted

I agree with Tif that if you are able to start recording we should be able to stop. 

Thanks
Hema

Comment 9

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Hema Koka [:hema] from comment #8)
> What happens when you unplug USB or press home button. Does the video stop
> recording then and save it? Adding qawanted

Unplugging USB cable stops the video and saves the video to internal storage. Pressing Home button does NOT stop recording, and if I go back to Camera it would show a black viewfinder and recording is not guaranteed to be saved after unplugging (I've seen it saved once after unplugging USB, but other times it just wouldn't save a video at all. Also the duration seems shorter than what I remembered recording).

In short, only unplugging USB cable will stop recording and save video, doing all other actions might make the video not save at all.

Device: Aries 2.5
BuildID: 20151028104739
Gaia: 2e89362de40a6c9c36525d36317fa1ae8e67e143
Gecko: fc706d376f0658e560a59c3dd520437b18e8c4a4
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 44.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Comment 10

[David Flanagan [:djf]]

I'll take this and see if I can get it fixed for 2.5

Comment 11

[David Flanagan [:djf]]

It turns out that the stop button is reacting, but (in the case when UMS is on) but it takes 10+ seconds to play the stop recording sound and hide the timer. The video does actually stop recording when you press the button, but things just don't respond well.

There is a similar problem for still photos.  The photo is taken right away, but there is a long (multiple seconds) delay before the thumbnail for the new video shows up in the preview circle on the bottom left.

This suggests to me that this is a gecko issue in the device storage or camera code.  I'll investigate a bit more, however.

Comment 12

[David Flanagan [:djf]]

I was hoping that I could change the camera to use getDeviceStorages()[0] instead of getDeviceStorage() and that by explicitly using the internal sdcard rather than using the generic one that defaults to it I would avoid whatever bug is slowing things down.  But I tried that and there was no change.

If there was some app other than camera that wrote to the internal storage while the external sdcard was mounted, I could see if that slows down. If so, then it is just device storage. If not, then maybe it is camera specific.

Comment 13

[David Flanagan [:djf]]

I ran adb shell top on the device during the 10 seconds between pressing the stop button and when it finally played the stop recording sound.  There did not seem to be any spike in CPU activity. So this is not an issue where writing a file is slow because the device is really busy doing something.

Comment 14

[David Flanagan [:djf]]

The ds-test app doesn't seem to be any slower when I run it with the external sdcard mounted, so I'm thinking that this issue is in the gecko camera code.

When the external sdcard is mounted for UMS, there is a long delay between the RecorderStateChagned "TrackCompleted" events and the RecorderStateChanged "Stopped" events, and the device storage change events don't arrive until after the camera Stopped event arrives.

I'm going to unassign myself from this and toss it over to Andrew.

Comment 15

[Andrew Osmond [:aosmond] (he/him)]

This isn't a gecko camera problem either. When you record normally, there are typically no media data chunks to flush to disk, presumably because I/O is reasonably fast. With the STR, I find there are usually 5 chunks which can take anywhere from 500ms to 2s to write each one. That's why it takes so long to stop the recording.

If we really want to still permit camera capturing in this situation, I think that we should change the UI flow to be more like taking a picture. The UI transitions after the picture is taken but before it is saved to disk. Once it is saved to disk, the thumbnail is displayed.

When recording, the TrackCompleted events signal there is no more true recording happening -- we just need to save what we already have completed to disk. We should transition the UI and stop the timer at this point, just like taking a picture. We can then generate/show the thumbnail when we get the Stopped event, which may have some delay.

However unlike taking a picture, the camera thread is still busy in this case, and so any new requests will be delayed until the writing finishes. Thus we should put up the spinner if it takes too long to prevent new inputs. These would all be app changes.

dhylands: My recommendations above for the app aside, it appears that merely *toggling* USB mass storage on and then off, and *then* recording is enough to reproduce this condition. Even if we expect writes to take longer with mass storage on (seems strange to me if the PC isn't actively using it), surely we don't expect it to be slower after it is disabled? A reboot of the device clears this condition.

Comment 16

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Andrew Osmond [:aosmond] from comment #15)
> dhylands: My recommendations above for the app aside, it appears that merely
> *toggling* USB mass storage on and then off, and *then* recording is enough
> to reproduce this condition. Even if we expect writes to take longer with
> mass storage on (seems strange to me if the PC isn't actively using it),
> surely we don't expect it to be slower after it is disabled? A reboot of the
> device clears this condition.

Addendum, didn't read the switch label, need to unplug it first, so I guess it was still in the mode hence why it remained slow. Mea culpa. Still, should it be slow?

Comment 17

[GitHub Autolander Bot]

Attachment: [gaia] aosmond:bug1206646 > mozilla-b2g:master

Comment 18

[Dave Hylands [:dhylands]]

I suspect that disabling and re-enabling UMS causes any cached pages to be dropped. So it would need to re-read the flash to repopulate directories, etc.

And there would probably be a scan of the volume taking place by the media db.

Comment 19

[David Flanagan [:djf]]

I agree that stopping the timer sooner is the right thing to do. But having a spinner up for 10s (or whatever) seems pretty bad. Given that this is a corner case and users will not normally want to use their camera during UMS, maybe we'd be better off just throwing up the "unplug your phone to use camera" overlay in this case, even though the internal storage isn't actually unmounted.

That is, it might be better to just make the camera unusable, with an explanation, in this situation rather than fight to make it barely usable.

Comment 20

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8683236 [details] [review]
[gaia] aosmond:bug1206646 > mozilla-b2g:master

LGTM. However, we may want to consider what djf said in Comment 19 if the user experience is that bad.

Comment 21

[Tiffanie Shakespeare [:tif]]

I agree with justindarc and djf - if we are actually showing a spinner for that long we should just prevent people from doing this. It's basically unusable.

Comment 22

[GitHub Autolander Bot]

Attachment: [gaia] aosmond:bug1206646-prevent > mozilla-b2g:master

Comment 23

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8689140 [details] [review]
[gaia] aosmond:bug1206646-prevent > mozilla-b2g:master

Okay, I've changed it to monitor all of the device storage objects and if any one of them is shared, it will put up the existing "unplug camera" dialog. Unfortunately we don't use mediadb so I had to implement that functionality again in camera.

Comment 24

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8689140 [details] [review]
[gaia] aosmond:bug1206646-prevent > mozilla-b2g:master

Sorry for the delay in reviewing this. Let's do one more round of revisions before landing. There's no "clean" way of doing what this patch needs to do, afaik, so let's try and tidy up some syntax where we can to make it a little more easier to read.

Comment 25

[BMO Automation]

Firefox OS is not being worked on