Closed Bug 1039943 Opened 10 years ago Closed 10 years ago

[tarako][dolphin][Gallery][mediadb]Gallery should stop scanning and parsing when app goes to background

Categories

(Firefox OS Graveyard :: Gaia::Gallery, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:1.4+, b2g-v1.3T affected, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed, b2g-v2.2 affected, b2g-master affected)

RESOLVED FIXED
2.1 S1 (1aug)
blocking-b2g 1.4+
Tracking Status
b2g-v1.3T --- affected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- affected
b2g-master --- affected

People

(Reporter: jinchao.wang, Assigned: djf)

References

()

Details

(Whiteboard: [partner-blocker][sprd332037][2.2-nexus-5-l])

Attachments

(8 files)

Preset Conditions: many new image files to phone (1)launch gallery from Homescreen (2)gallery will scanning and parsing new image file first (3)when scanning and parsing, gallery goes to background,but scanning and parsing doesn't pause. When background app scanning and parsing ,switching applications will be not smooth,especially in the low-profile phones. music and gallery have the same issue。 SPRD provide a patch : music & gallery pause scanning when they go to background,and music & gallery resume scanning when they resume.
blocking-b2g: --- → 1.4?
Priority: -- → P1
Flags: needinfo?(ttsai)
Flags: needinfo?(fabrice)
Summary: [FFOS v1.4][Gallery][mediadb]Gallery should stop scanning and parsing when app goes to background → [tarako][dolphin][Gallery][mediadb]Gallery should stop scanning and parsing when app goes to background
Whiteboard: [partner-blocker][sprd332037]
David, does that sound reasonable to you?
Flags: needinfo?(fabrice) → needinfo?(dflanagan)
(In reply to Fabrice Desré [:fabrice] from comment #2) > David, does that sound reasonable to you? The patch looks good, though I'd have to study it more carefully before I was willing to land it. I think, though, that pausing a scan is not the right thing to do here. I'd prefer to abort the scan and restart it from scratch. If a media app scan is paused when the app goes to the background and the user then copies new files to the phone by UMS, the gallery will start a new scan in the background while there is already a paused scan. That seems like it will cause problems. And if the user uses the camera to take new pictures while there is a paused gallery scan pending, the gallery will end up updating this metdata database while it has a pending scan, and that also seems like it could cause bad problems. Given the memory constraints on Tarako, especially, Gallery and Music are likely to be killed while they are in the background and the scans will never resume anyway. So in that case, just aborting the scan won't hurt anything.
Flags: needinfo?(dflanagan)
Flags: needinfo?(ttsai)
Jingchao, The concept makes sense but we would prefer to go with David's suggestion per comment 3 - Abort and restart. Can you change your patch to doing this? Thanks
Assignee: nobody → jinchao.wang
blocking-b2g: 1.4? → 1.4+
Flags: needinfo?(jinchao.wang)
Flags: needinfo?(jinchao.wang)
(In reply to Wayne Chang [:wchang] from comment #4) > Jingchao, > > The concept makes sense but we would prefer to go with David's suggestion > per comment 3 - Abort and restart. > Can you change your patch to doing this? > > Thanks According to your suggestion, I updated my patch. Can help evaluating the patch?
(In reply to David Flanagan [:djf] from comment #3) > (In reply to Fabrice Desré [:fabrice] from comment #2) > > David, does that sound reasonable to you? > > The patch looks good, though I'd have to study it more carefully before I > was willing to land it. > > I think, though, that pausing a scan is not the right thing to do here. I'd > prefer to abort the scan and restart it from scratch. If a media app scan is > paused when the app goes to the background and the user then copies new > files to the phone by UMS, the gallery will start a new scan in the > background while there is already a paused scan. That seems like it will > cause problems. > > And if the user uses the camera to take new pictures while there is a paused > gallery scan pending, the gallery will end up updating this metdata database > while it has a pending scan, and that also seems like it could cause bad > problems. > > Given the memory constraints on Tarako, especially, Gallery and Music are > likely to be killed while they are in the background and the scans will > never resume anyway. So in that case, just aborting the scan won't hurt > anything. Hi David : Although I provide a patch ,I prefer that you be able to give a solution , my patch is only a reference ,or a discussion. Can you provide us with a solution based on my two patch ?
Flags: needinfo?(dflanagan)
David, Can you please provide your input? Thanks Hema
Target Milestone: --- → 2.1 S1 (1aug)
In the long term, I think we can find a good solution to this problem. Because of video hardware limitations, the video app scans files in the background but only creates video thumbnails when it is in the foreground. Gallery could be modified to do that same thing, and bug 1028751 might be a good place to address that. But for this 1.4+ bug we need something quicker. Spreadtrum proposed a patch that would pause the mediadb scan when the gallery goes into the background and then restart it when it comes back to the foreground. For the reasons given in comment #3 I think this could cause data corruption issues. I proposed instead that we actually fully abort the scan if the gallery goes to the background and then resume it when we come back to the foreground. Even that makes me nervous, though and I worry about the testing required to ensure that it does not break things. For example, I'm not certain if taking a new photograph while the gallery is in the background would update the most recent known photo date. If it did, then when the app came back to the foreground, the scan would be started to look for photos newer than that most recent photo, and it would not quickly find older photos the way it should. So my idea now is that if the gallery goes to the background while it is still scanning, maybe it should just exit completely. On a memory constrained device (Tarako at least), it is likely that it would be killed while in the background anyway. And as bug 1028751 notes, the gallery is hard to use while the scanning process is going on because the thumbnails keep jumping around. So it is likely that the user will stay in thumbnail view while the scan is happening anyway. So if we kill and restart the app, they won't really even notice, except for the somewhat longer startup time to reload all the existing thumbnails. Since we don't allow the user to edit images while a scan is going on, so there is no possibility of the user losing an edit-in-progress if we quit during a scan. I'm taking this bug and will create a simple patch to quit if we go to the background before the scan has completed. Jingchao: does this seem like an acceptable solution?
Assignee: jinchao.wang → dflanagan
Flags: needinfo?(dflanagan) → needinfo?(jinchao.wang)
Depends on: 1046995
Attached file link to master branch patch on github (deleted) —
Punam: what do you think of this approach of just exiting? Can we land this in master and 2.0 and then fix it for real in bug 1046995?
Attachment #8465736 - Flags: review?(pdahiya)
Attached file pull request for v1.4 (deleted) —
Jingchao, Does this patch fix the bug satisfactorily for you?
Attachment #8465739 - Flags: review?(jinchao.wang)
(In reply to David Flanagan [:djf] from comment #11) > Created attachment 8465739 [details] > pull request for v1.4 > > Jingchao, > > Does this patch fix the bug satisfactorily for you? Hi David: Thanks for your patch. I have reviewed the patch and it looks good. Besides,Can you do the same modifications for music? Or I submit another bug to trace music issue?
Flags: needinfo?(jinchao.wang)
Hi David, with this patch, may i know the expected result on following scenarios? 1. First time boot up 2. Open Music app, and start scanning songs (1st time scanning) 3. user press play once he saw the 1st album / song title. 4. play music and push music app to background. if systems scanned 3 songs and stop scanning after pushed to background, so user can only listen 3 songs?
Flags: needinfo?(dflanagan)
Comment on attachment 8465736 [details] link to master branch patch on github Hi David The patch looks good, I like that we are exiting only for low mem devices and only while scanning, I tested in master and has r+. Thanks
Attachment #8465736 - Flags: review?(pdahiya) → review+
(In reply to Marvin Khoo [:Marvin_Khoo] from comment #13) > Hi David, > with this patch, may i know the expected result on following scenarios? > > 1. First time boot up > 2. Open Music app, and start scanning songs (1st time scanning) > 3. user press play once he saw the 1st album / song title. > 4. play music and push music app to background. > > > if systems scanned 3 songs and stop scanning after pushed to background, so > user can only listen 3 songs? Marvin: this patch only affects Gallery. I only just now noticed that there was any discussion of the Music app in the bug at all. If changes are needed for the Music app, we should do that in a separate bug.
Flags: needinfo?(dflanagan)
(In reply to jingchao.wang from comment #12) > Besides,Can you do the same modifications for music? Or I submit another > bug to trace music issue? Please file a separate bug for Music. Perhaps you can use the code from this Gallery patch and apply it to the Music app. But note Marvin's question in comment #13: we can only force the Music app to exit if the user has not yet started playing music, of course.
I've updated the PR to activate this hack on any device with less than 512mb rather than on any device that has less than or equal to 256mb, and am carrying the r+ forward. This change is motivated by the discussion in the dev-gaia thread "low-memory code paths", and also by the desire to be able to have this new behavior manifest on our low-memory Flames which get configured to have memory between 256 and 512. I don't expect that this change will affect any production devices which are likely to have 256 or 512 and not use values in-between.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
But what about music background scanning? Jinchao's patch include stopping music background scanning.
Flags: a11y-review+
Comment on attachment 8465739 [details] pull request for v1.4 R+ ,it's looks good to me.
Attachment #8465739 - Flags: review+
(In reply to James Zhang (Spreadtrum) from comment #21) > But what about music background scanning? Jinchao's patch include stopping > music background scanning. Hi James: I have fired another bug1048085 to trace music issue.
Depends on: 1048085
Flags: in-moztrap?(bzumwalt)
New test case needs to be written to address bug.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Test case has been added to moztrap: https://moztrap.mozilla.org/manage/case/14373/
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(bzumwalt)
Flags: in-moztrap+
Attachment #8465739 - Flags: review?(jinchao.wang) → review+
This issue CAN be repro on Nexus 5_v2.2&3.0. Reproduce rate:5/5 N5_3.0(affected): Build ID 20150317160205 Gaia Revision 63d6639acd771f548a2613f07f3e335921e4ac87 Gaia Date 2015-03-17 16:53:50 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/e965a1a534ec Gecko Version 39.0a1 Device Name hammerhead Firmware(Release) 5.0 Firmware(Incremental) eng.cltbld.20150317.194723 Firmware Date Tue Mar 17 19:47:37 EDT 2015 Bootloader HHZ12d N5_2.2(affected): Build ID 20150317162504 Gaia Revision 306772a58335ac4cad285d27c3805090a8cc6886 Gaia Date 2015-03-17 17:12:36 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/83251e534b33 Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.0 Firmware(Incremental) eng.cltbld.20150317.195033 Firmware Date Tue Mar 17 19:50:47 EDT 2015 Bootloader HHZ12d
Whiteboard: [partner-blocker][sprd332037] → [partner-blocker][sprd332037][2.2-nexus-5-l]
regression? I checked the code that "doNotScanInBackgroundHack" still in 2.2. -- Keven
Coler, is this still reproducible on 2.2 and master?
Flags: needinfo?(liuyong)
Hi, Hermes About Gallery, this bug is still reproducible on latest build of Nexus5_2.2&3.0 by STR as comment 0, but is NOT on Flame2.2&3.0. About Music, it is still reproducible on latest build of Nexus5_2.2&3.0, Flame2.2&3.0 by STR as comment 0. Actual result: On Nexus 5_2.2&3.0, it will continue scanning and parsing in Gallery and Music when app goes to background. On Flame 2.2&3.0, Only Music will continue scanning and parsing when app goes to background. See attachment:Reproduce_Gallery.3gp, Reproduce_Music.3gp, logcat_Gallery_2044.txt, logcat_Music_2211.txt Found time: Music: 22:11 Gallery: 20:44 Reproducing rate: Nexus 5_2.2&3.0(Gallery&Music): 5/5 Flame 2.2&3.0 Music:5/5 Flame 2.2&3.0 Gallery:0/5 Device:Nexus5_3.0 build(Affected): Build ID 20150616160206 Gaia Revision 9d42e94446f2dc01b519172b6d75d81d4d435bdc Gaia Date 2015-06-16 16:16:47 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/27caa5299f9f Gecko Version 41.0a1 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150616.193221 Firmware Date Tue Jun 16 19:32:40 EDT 2015 Bootloader HHZ12f Device:Flame 2.2 build([Music]Affected): Build ID 20150616002505 Gaia Revision e7a0c6d5f4df04d45fb3f726efb9e8223600cb79 Gaia Date 2015-06-15 06:12:18 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8045028bf400 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150616.035412 Firmware Date Tue Jun 16 03:54:24 EDT 2015 Bootloader L1TC000118D0 Device:Nexus5_2.2 build(Affected): Build ID 20150616002505 Gaia Revision e7a0c6d5f4df04d45fb3f726efb9e8223600cb79 Gaia Date 2015-06-15 06:12:18 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8045028bf400 Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150616.034849 Firmware Date Tue Jun 16 03:49:07 EDT 2015 Bootloader HHZ12f Device:Flame 3.0 build([Music]Affected): Build ID 20150616010205 Gaia Revision 62ba52866f4e5ca9120dad5bfe62fc5df981dc39 Gaia Date 2015-06-15 19:09:24 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/ce863f9d8864 Gecko Version 41.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150616.060924 Firmware Date Tue Jun 16 06:09:36 EDT 2015 Bootloader L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+],[MGSEI-Triage+]
Flags: needinfo?(liuyong) → needinfo?(hcheng)
Attached file logcat_Music_2211.txt (deleted) —
Attached file logcat_Gallery_2044.txt (deleted) —
Attached video Reproduce_Gallery.3gp (deleted) —
Attached video Reproduce_Music.3gp (deleted) —
Hi Coler, please clone this bug for following up. Thanks.
Flags: needinfo?(hcheng) → needinfo?(liuyong)
(In reply to Hermes Cheng[:hermescheng] from comment #34) > Hi Coler, please clone this bug for following up. Thanks. Hi Hermes, I have filed a new bug 1175456.
Flags: needinfo?(liuyong)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: