Bugzilla

Comment 3

•

10 years ago

Removing the sdcard while the flame is powered isn't supported.

Having said that, we probably shouldn't crash either.

Hema Koka [:hema]

Comment 4

•

10 years ago

Russ is helping investigate

Assignee: nobody → rnicoletti

Comment 5

•

10 years ago

I'm not able to reproduce using the following environment:

application_buildid: 20140930142714
application_display_name: B2G
application_name: B2G
application_repository: 43a3a6cc35be963f5c987b6d121e8d14a593e9df
application_version: 32.0
device_firmware_date: 1412057196
device_firmware_version_incremental: 34
device_firmware_version_release: 4.4.2
device_id: flame-kk
gaia_changeset: 1dc2e29491da234cfa461916304d77ce88b50045
Firmware: v184

The behavior I'm seeing when the sdcard is not present is that some videos while playing will at some point return to the beginning.

I will try with v180 to see if there is a difference in behavior.

Comment 6

•

10 years ago

Attached file v180, 319mem logcat — Details

I'm also not seeing a crash with firmware v180:

application_buildid: 20140904160718
application_display_name: B2G
application_name: B2G
application_repository: 2b27becae85092d46bfadcd4fb5605e82e1e1093
application_version: 32.0
device_firmware_date: 1409813942
device_firmware_version_incremental: 27
device_firmware_version_release: 4.4.2
device_id: flame
gaia_changeset: 506da297098326c671523707caae6eaba7e718da
gaia_date: 1409816577
platform_buildid: 20140904160718

The behavior I'm seeing with v180 is that certain videos do not play at all when the sd card is removed. But the video app is not crashing.

Logcat is attached.

Hema Koka [:hema]

Comment 7

•

10 years ago

cnelson, 

Russ is not able to reproduce this crash. Please re-check if there is a different STR to get to this crash or if this is issue works now with recent build.

Thanks
Hema

Flags: needinfo?(cnelson)

Comment 8

•

10 years ago

What the video is showing is that MediaDB enumerates its database and displays thumbanils for everything it finds before it starts the scanning process and discovers that some of those videos are no longer there. This bug has been filed elsewhere as a privacy issue: it also occurs for videos that are deleted by UMS. There are similar issues in Gallery.

Punam has investigated a fix in Gallery that checks for the existance of each photo or video during the thumbnail enumeration process. There's a performance impact and we nave not landed that patch. A simpler fix could address this issue, however: files for which no matching storage area is available should not be enumerated. This is probably something that should be done in MediaDB eventually, but could also be done in the individual apps for a temporary fix.

Note that this problem can only occur on devices, like flame, that have more than one storage area. Otherwise, with the card out we would not let the user run the app at all.

This bug might also be reproducable if you have a lot of videos on the sdcard and pull it while the app is running. There may be time to start playing one of the videos before the scanning process discovers that it is now gone.

If we can reproduce the crash then this should really be a blocker.

cnelson

Reporter

Comment 9

•

10 years ago

Note that currenty, the videos will play for several seconds before jumping back to the start when the sd card is not plugged in.  However, if the user pulls the sd card during the video a crash will occur.

Flags: needinfo?(cnelson)

Hema Koka [:hema]

Comment 10

•

10 years ago

Blocking Reason: Crash leading to poor user experience.


Russ, please work with No-jun or cnelson to reproduce this issue and see whats going on. Dave tells me that it might be a tricky one. If you can investigate and narrow down, Dave could help out once he is done with other blockers that he has. 

Thanks
Hema

blocking-b2g: 2.1? → 2.1+

Comment 11

•

10 years ago

I took a look at the crashdump from comment 0, and basically, this is crashing inside OMX.

So the crash is happening inside the android code, and isn't part of gecko or gaia.

It's actually an assert firing right here:
https://github.com/mozilla-b2g/platform_frameworks_av/blob/master/media/libstagefright/matroska/MatroskaExtractor.cpp#L274

Which makes this part of vendor build.

Comment 12

•

10 years ago

Attached file bug-1077642.patch — Details

From the user’s perspective, it appears there are two separate scenarios:

1) Playing a video after the sd card has been pulled out
2) Playing a video while the sd card is pulled out

From my investigation, they are the same from the app's perspective. In both cases, the app gets a 'cardremoved' event [1], which results in the app hiding the video and returning to list view. When hiding the video, the app is also attempting to update the video metadata (for example, to record the current position of the video so that it can update the thumbnail image). The process of updating the metadata involves reading the file from the db. When the sd card has been removed, the read transaction receives a ‘success' event but the 'result' of the read is undefined. This scenario, the result of the read being undefined in context of a ’success' event, is not handled by the app and can cause a crash. I have attached a patch that changes the behavior such that the video app does not update metadata when the sd card is removed (which seems reasonable to me).

[1] In the first case, the event comes when the video starts playing; in the second case, it comes when the sd card is pulled out.

cnelson, can you apply the patch, install the patched video app, and try the test again?

Flags: needinfo?(cnelson)

Comment 13

•

10 years ago

No-jun, can you also test the patch? Thanks.

Flags: needinfo?(nojun.park)

Comment 14

•

10 years ago

(In reply to Dave Hylands [:dhylands] from comment #11)
> I took a look at the crashdump from comment 0, and basically, this is
> crashing inside OMX.
> 
> So the crash is happening inside the android code, and isn't part of gecko
> or gaia.
> 
> It's actually an assert firing right here:
> https://github.com/mozilla-b2g/platform_frameworks_av/blob/master/media/
> libstagefright/matroska/MatroskaExtractor.cpp#L274
> 
> Which makes this part of vendor build.

I'm not able to reproduce this crash. I'm consistently seeing the behavior described in comment 12.

Comment 15

•

10 years ago

(In reply to Russ Nicoletti [:russn] from comment #12)

> From my investigation, they are the same from the app's perspective. In both
> cases, the app gets a 'cardremoved' event [1], which results in the app
> hiding the video and returning to list view. When hiding the video, the app
> is also attempting to update the video metadata (for example, to record the
> current position of the video so that it can update the thumbnail image).
> The process of updating the metadata involves reading the file from the db.

I think you mean reading the file from device storage, right? Because we're creating the new poster image?

> When the sd card has been removed, the read transaction receives a ‘success'
> event but the 'result' of the read is undefined. 

This is the device storage read transaction? (i.e. a mediadb.getFile()?)  Or do you really mean an indexedDB read?

This scenario, the result
> of the read being undefined in context of a ’success' event, is not handled
> by the app and can cause a crash. 

That seems like a gecko bug there. We shouldn't get a success with no result.  But if we're talking gaia level code, it wouldn't be responsible for a crash anyway.

> I have attached a patch that changes the
> behavior such that the video app does not update metadata when the sd card
> is removed (which seems reasonable to me).

Nice catch! I think we should land this patch even if it does not resolve this particular crash.

Comment 16

•

10 years ago

Comment on attachment 8502067 [details]
bug-1077642.patch

Yes, the updateMetadata function is performing an indexedDB read from device storage to create a new poster image.

Attachment #8502067 - Flags: review?(dflanagan)

cnelson

Reporter

Comment 17

•

10 years ago

Crash still occurs after applying the patch.

Environmental Variables:
Device: Flame 2.1
BuildID: 20141008000201
Gaia: 55ce3612bd8a8665139d6b85114ce59993a3fa0a
Gecko: 7fa82c9acdf2
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flags: needinfo?(cnelson)

Comment 18

•

10 years ago

(In reply to Russ Nicoletti [:russn] from comment #16)

> Yes, the updateMetadata function is performing an indexedDB read from device
> storage to create a new poster image.

That sentence doesn't make any sense! IndexedDB and DeviceStorage are completely different things. And IndexedDB never touches the SD card so wouldn't be affected in this bug. MediaDB is a wrapper that uses both IndexedDB and DeviceStorage.  

I know that when we stop playing a video, we create a new thumbnail that shows the current playback location. Does that happen in the call to updateMetadata() or somewhere else? In the cardremoved case, we presumably want to prevent that new thumbnail from being created because with the card gone, we can't trust that it will succeed. 

Does your patch actually prevent the new thumbnail, or is that happening somwhere else?

Flags: needinfo?(rnicoletti)

Comment 19

•

10 years ago

Regarding the sentence that doesn't make any sense, here is where the read is initiated: https://github.com/mozilla-b2g/gaia/blob/master/shared/js/mediadb.js#L1016

Yes, the patch prevents the new thumbnail from being created, that happens in the 'updateMetadata' function in video.js (line 1081). This function is not called when 'hidePlayer' is called with 'false' (don't update metadata).

Flags: needinfo?(rnicoletti)

Comment 20

•

10 years ago

The other thing that happens when we pull a card out while a video is playing is that we return to the list of videos and, if we have not processed all the metadata for all known videos, we resume doing that. If we have scanned the sdcard and have videos that still need to be processed, then it is possible that we're causing problems by trying to scan videos that are not there anymore.  Obviously there still should not be a crash, but maybe there is something we can do here that can work around the crash. But I'm straying farther from the original scenario pictured in the video...

Actually, thinking back to that video, it just doesn't make sense that a video could play at all if the sdcard had been removed.  Here's the only scenario I can think of that would allow that video to be created.

0) Put a video on the internal storage of the device. Start the video app and let it scan. Kill the video app.
1) put a lot of videos on the sdcard.
2) start the video app and allow it to scan the card, but don't give it time to create poster images for all the videos.
3) return to the homescreen. The video app will stop processing poster images while it is in the background.
4) This is when the youtube video recording starts for this bug.
5) Remove the sdcard
6) Launch the already-running video app. When it comes to the foreground, it will start trying to create a poster image for one of the videos on the sdcard.
7) Meanwhile, tap on the thumbnail for the video on internal storage and it starts playing
8) And at this point, the process of trying to create a thumbnail for the video that is no longer there fails with a crash.

This seems like kind of a stretch. But if the problem is something like this, then maybe a solution is that when we go through our list of files that need thumbnails, we should always check that the device storage object is actually still there before trying to parse the metadata.

Having said all that, I would not consider this a very high-priority bug because users are unlikely to remove the sdcard while apps are running!

Comment 21

•

10 years ago

(In reply to Russ Nicoletti [:russn] from comment #12)

> When the sd card has been removed, the read transaction receives a ‘success'
> event but the 'result' of the read is undefined. 

This does seem quite mysterious.  It could be a regression from 994190. Also, however, when the sdcard is pulled, the mediadb starts updating itself to remove records for all of the videos on the sdcard. So maybe this is just just means that mediadb removed the record before updateMetadata read the record.  I'd have to go back to the IndexedDB docs to find out what is normal when you try to read a record that does not exist. Maybe success with an undefined value is actually the expected thing in that case.

Comment 22

•

10 years ago

Comment on attachment 8502067 [details]
bug-1077642.patch

This patch is now associated with bug 1081386.

Attachment #8502067 - Flags: review?(dflanagan)

Comment 23

•

10 years ago

Updating component based on comment 11

Assignee: rnicoletti → nobody

Component: Gaia::Video → Vendcom

Comment 24

•

10 years ago

Bug 1081386 has been created to address the issue with updating video metadata after the sd card has been removed while the app is running.

Wesley Huang [:wesley_huang] (EPM) (NI me)

Comment 25

•

10 years ago

ni- no-jun as it's clear the patch doesn't affect the crash.

Flags: needinfo?(nojun.park)

Comment 26

•

10 years ago

Hi Wesly,
Can you raise this to partner?

Flags: needinfo?(wehuang)

Comment 27

•

10 years ago

Youlong: would you help check this issue? Though some unusual use case to me (pull SD while playing video) but I expect no crash at any situation. Thanks.

Flags: needinfo?(wehuang) → needinfo?(zhuyoulong)

youlong.jiang

Comment 28

•

10 years ago

(In reply to Wesly Huang from comment #27)
> Youlong: would you help check this issue? Though some unusual use case to me
> (pull SD while playing video) but I expect no crash at any situation. Thanks.

hi wesly -

the crash you mentioned means app or system? if app crash under this status, I think this is normal or need up-layer to handle

tks.

Comment 29

•

10 years ago

Hi Youlong, I think there is some clue in comment#11, but suggest you still read through all comments to understand the investigation/work that has been done.

Flags: needinfo?(youlong.jiang)

youlong.jiang

Comment 30

•

10 years ago

(In reply to Wesly Huang from comment #29)
> Hi Youlong, I think there is some clue in comment#11, but suggest you still
> read through all comments to understand the investigation/work that has been
> done.

hi wesly -

I also could not understand what we can do for this issue.
in my opinion, this bug is absolutely related to logic check problem. the error or crash is normal when under unusual case. to comment#11, you mentioned OMX report some crash log, but I think this problem is not caused by decode/encode, so pls take a double confirm.

tks.

Flags: needinfo?(youlong.jiang)

Comment 31

•

10 years ago

What's happening is that because the HW has no sdcard removed signal, the kernel doesn't know that the sdcard was removed.

The OMX decoder is reading the file, and eventually there is no more data cached. The kernel tries to read from the non-existant sdcard, and gets an error. The OMX decoder didn't get the data it expected, so it asserts.

That assert is the crash that's being seen.

I'm going to guess that I can construct a corrupt file which would cause the same assert to occur without having to remove the sdcard.

youlong.jiang

Comment 32

•

10 years ago

(In reply to Dave Hylands [:dhylands] from comment #31)
> What's happening is that because the HW has no sdcard removed signal, the
> kernel doesn't know that the sdcard was removed.
> 
> The OMX decoder is reading the file, and eventually there is no more data
> cached. The kernel tries to read from the non-existant sdcard, and gets an
> error. The OMX decoder didn't get the data it expected, so it asserts.
> 
> That assert is the crash that's being seen.
> 
> I'm going to guess that I can construct a corrupt file which would cause the
> same assert to occur without having to remove the sdcard.

Dear Hylands -

I really agree with your summarize. then the root case is data source input nor decode process, and I don't think it's a good idea to fix this bug in OMX. 

tks.

Bobby Chien

Comment 33

•

10 years ago

In my 2 Flame 2.1 and 2.2. Video app didn't crashed, but video clip is stopped instead. I use image v188.

----------

Build ID              20141103161201
Gaia Revision         903f6805ec3959c6bdb84d44100981dbac3903cb
Gaia Date             2014-11-03 18:03:23
Gecko Revision        https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f3d1ad0fcd64
Gecko Version         34.0
Device Name           flame
Firmware(Release)     4.4.2
Firmware(Incremental) 39
Firmware Date         Thu Oct 16 18:19:14 CST 2014
Bootloader            L1TC00011880

Build ID              20141109160204
Gaia Revision         5f8206bab97cdd7b547cc2c8953cadb2a80a7e11
Gaia Date             2014-11-07 19:08:20
Gecko Revision        https://hg.mozilla.org/mozilla-central/rev/d380166816dd
Gecko Version         36.0a1
Device Name           flame
Firmware(Release)     4.4.2
Firmware(Incremental) 39
Firmware Date         Thu Oct 16 18:19:14 CST 2014
Bootloader            L1TC00011880

Comment 34

•

10 years ago

Hi Dave: 

Thanks for your summary in comment#31 and just 1 stupid question from me.

Per that comment it all starts w/ "...HW has no sdcard removed signal, the kernel doesn't know that the sdcard was removed...", is this precondition reasonable? I mean are we expecting there is no "sdcard removal signal" from HW to kernel?

Thanks in advance.

Flags: needinfo?(zhuyoulong) → needinfo?(dhylands)

Comment 35

•

10 years ago

There are two different styles of sdcard connector.

The one used on the flame was not designed for hot-swapping. Technically, the reason for this is that the pins come in contact with the sdcard in an unpredictable order, and so you can't assure a proper power-up sequence for the card. Since the connector isn't designed for hot-swapping, there is no need for a media insertion signal, and the signal is just not present.

The other style of connector is the slide-in slide-out (used on the dolphin). This style of connector can enforce that the pins come in contact in a prescribed order, and then hot-swapping can give reliable powerup of the card. These types of connectors typically have a "media present" signal.

So, to summarize, you can only hot-swap the sdcard if the sdcard connector that's used was actually designed to support hot-swapping. Unfortunately, the physical sdcard connector found on the flame was not designed for hot-swapping.

Flags: needinfo?(dhylands)

Lancy(Leave from Mozilla)

Comment 36

•

10 years ago

Thanks Dave's sharing, I learned a lot!

So my understanding now is, due to this Flame HW design limitation (the SDcard slot's style), there is no good way to fix this issue in low level, like mentioned in comment#31 & comment#32.

bhavana bajaj [:bajaj]

Comment 37

•

10 years ago

(In reply to Wesly Huang from comment #36)
> Thanks Dave's sharing, I learned a lot!
> 
> So my understanding now is, due to this Flame HW design limitation (the
> SDcard slot's style), there is no good way to fix this issue in low level,
> like mentioned in comment#31 & comment#32.

Given this I'll unblock and we will forward this to Asa when selecting the new reference hardware. We'd have to keep an eye on devices outside of Flames getting this crash though. So if QA or device team have other devices to try and reproduce, we will take it form there-on.

blocking-b2g: 2.1+ → -

Tony Chung [:tchung]

Updated

•

10 years ago

Summary: crash in pthread_kill → [Flame] crash in pthread_kill

Comment 38

•

10 years ago

Attached file logcat_0331.txt — Details

This issue still exist on the Flame 2.1.
Issue steps:
1.Launch Camera.
2.Record a video by front camera
3.Play the video in preview mode.
4.Frequently adjust the progress.

Expected Result:Video can play without problem.
Actual Result:Camera crashed.

Attach the video:Video.mp4.
Attach the logcat:logcat.txt.
Occurrence time:03:31
Occurrence rate:5/10

Flame v2.1 version:
Gaia-Rev        afdfa629be209dd53a1b7b6d6c95eab7077ffcd9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dc3018cbdbe6
Build-ID        20141123001201
Version         34.0