Closed
Bug 1074604
Opened 10 years ago
Closed 10 years ago
[MTP][KK] PC could not sync video which is recorded by device when MTP enable and USB plugged in
Categories
(Firefox OS Graveyard :: MTP/UMS, defect)
Tracking
(blocking-b2g:2.1+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.1 verified, b2g-v2.2 verified)
People
(Reporter: ashiue, Assigned: edenchuang)
Details
(Whiteboard: [ft:Peripherals])
Attachments
(1 file, 3 obsolete files)
Gaia-Rev 13973ab50760d1e8bb773082163f0dff19d35a44 Gecko-Rev https://hg.mozilla.org/releases/mozilla-aurora/rev/6e317e075d04 Build-ID 20140928160204 Version 34.0a2 STR: 1. USB plugged in and enable MTP 2. Device record an video Expect result: 1. The video shows on PC Actual result: 1. The video does not show on PC (only generate an image which has the same name as the video on PC)
Reporter | ||
Comment 1•10 years ago
|
||
[Blocking Requested - why for this release]: functional issue
blocking-b2g: --- → 2.1?
QA Whiteboard: [COM=Storage]
Comment 3•10 years ago
|
||
Triage: blocker.
Assignee: nobody → echuang
blocking-b2g: 2.1? → 2.1+
Whiteboard: [ft:Peripherals]
Updated•10 years ago
|
QA Contact: ckreinbring
Comment 4•10 years ago
|
||
The bug repros on Flame 2.2 and Flame 2.1 using shallow flash. Actual result: With MTP enabled, no file appears on the PC after recording a video from the Camera app. Flame 2.2 BuildID: 20141003070740 Gaia: a8a6eed2ba9d66239aac789b9ee4900f911c73cb Gecko: 388e101e75c8 Platform Version: 35.0a1 Firmware Version: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Flame 2.1 BuildID: 20141003074236 Gaia: 88f431fcf75f274d9db66b9d3c8af7b6dc29edb1 Gecko: 0272a82658ff Platform Version: 34.0a2 Firmware Version: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 -------------------------------------------------------------------------------------------------------- The bug cannot be reproed on Flame 2.0 due to the fact that it does not have MTP functionality. BuildID: 20141003065235 Gaia: 092d2b7678774c8b0b06dca0e0a8119e9eafdec3 Gecko: 43e53f774179 Platform Version: 32.0 Firmware Version: V180 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [COM=Storage] → [QAnalyst-Triage?][COM=Storage]
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → affected
Flags: needinfo?(jmitchell)
Keywords: qawanted
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?][COM=Storage] → [QAnalyst-Triage+][COM=Storage]
Flags: needinfo?(jmitchell)
Assignee | ||
Comment 5•10 years ago
|
||
Following is the logcat of the case to generate vedio VID_0006.3gp 1032 10-06 05:26:37.980 5490 5490 I MozMtp : Observe: file-watcher-update: file DCIM/100MZLLA/tmp.3gp created 1033 10-06 05:26:37.980 5490 6019 I MozMtp : FileWatcherUpdate: file /storage/sdcard/DCIM/100MZLLA/tmp.3gp created 1034 10-06 05:26:37.980 5490 6019 I MozMtp : FileWatcherUpdate: About to call sendObjectAdded Handle 0x00000006 file /storage/sdcard/DCIM/100MZLLA/tmp.3gp 1035 10-06 05:26:37.980 5490 5490 I Gecko : DSF (parent) file-watcher-update: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/tmp.3gp' mFile->GetPath '/storage/sdcard/DCIM/ 100MZLLA/tmp.3gp' 1036 10-06 05:26:37.980 5490 5490 I MozMtp : Observe: file-watcher-update: file DCIM/100MZLLA/tmp.3gp modified 1037 10-06 05:26:37.980 5490 6019 I MozMtp : FileWatcherUpdate: file /storage/sdcard/DCIM/100MZLLA/tmp.3gp modified ... 1062 10-06 05:26:38.070 6029 6029 I Gecko : DSF (child) DeviceStorageCreateFdParams: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/VID_0006.3gp' mFile->GetPath '/storage/ sdcard/DCIM/100MZLLA/VID_0006.3gp' 1063 10-06 05:26:38.070 5490 5490 I Gecko : DSF (parent) file-watcher-update: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/tmp.3gp' mFile->GetPath '/storage/sdcard/DCIM/ 100MZLLA/tmp.3gp' 1064 10-06 05:26:38.070 5490 5490 I MozMtp : Observe: file-watcher-update: file DCIM/100MZLLA/tmp.3gp deleted 1065 10-06 05:26:38.070 5490 6019 I MozMtp : FileWatcherUpdate: file /storage/sdcard/DCIM/100MZLLA/tmp.3gp deleted 8062 10-06 05:26:50.660 5490 5490 I Gecko : DSF (parent) file-watcher-update: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/VID_0006.3gp' mFile->GetPath '/storage/sdcard/ DCIM/100MZLLA/VID_0006.3gp' ... 8063 10-06 05:26:50.660 5490 5490 I MozMtp : Observe: file-watcher-update: file DCIM/100MZLLA/VID_0006.3gp modified 8064 10-06 05:26:50.660 5490 6019 I MozMtp : FileWatcherUpdate: file /storage/sdcard/DCIM/100MZLLA/VID_0006.3gp modified According to the logcat, we found that MtpServer never receive the create event for VID_0006.3gp. It seems that because VID_0006.3gp is renamed from tmp.3gp. MtpServer got the tmp.3gp "delete" event(see line 1065, and at line 1064, we can know the file VID_0006.3gp exists), but MtpServer does not know that is caused by file renaming. At last, Camera app will finish VID_0006.3gp generation, then send "modified" event(line 8064). At the tmp.3gp delete point, MtpServer has no idea it is renamed to VID_0006.3gp, and cannot handle this proper. Fortunately, Camera will send "modified" event while finish VID_0006.3gp generation. If MozMtpServer receive a "modified" event but cannot find it file entry in database, MozMtpDatabase need to create the entry for the file. In fact, we need to update MozMtpDatabase properly for every "modified" event. Bug 1074600 is the case that we only create the entry in MozMtpDatabase but without update it while we receive "modified" event. It makes the file information be incomplete.
Attachment #8500332 -
Flags: feedback?(dhylands)
Comment 6•10 years ago
|
||
Comment on attachment 8500332 [details] [diff] [review] Handle "modified" event in MozMtpServer and MozMtpDatabase Review of attachment 8500332 [details] [diff] [review]: ----------------------------------------------------------------- I would also change endSendObject so that it only calls FileWatcherNotify(entry, "modified"); This means we can remove mBeginSendObjectCalled entirely. So I think you're on the right track, but with these suggestions thing should work much better. ::: dom/system/gonk/MozMtpDatabase.cpp @@ +296,5 @@ > RemoveEntry(entryHandle); > return; > } > + > + if (aEventType.EqualsLiteral("modified")) { After working on bug 1072535, I think that we should ignore "created" events, and treat "modified" like we used to treat created. The mediadb ignores "created" and only processes "modified". Camera, Bluetooth and Download only generate modified events. Within device storage, the created event is sent when the file is created (and has zero size). The modified event is sent when the data has been written. So I think it makes sense to not tell MTP about the file until we get the modified event. So I would remove this and change "created" above (around line 270) to be "modified". ::: dom/system/gonk/MozMtpServer.cpp @@ +107,5 @@ > return NS_OK; > } > > NS_ConvertUTF16toUTF8 eventType(aData); > + if (!eventType.EqualsLiteral("created") && !eventType.EqualsLiteral("deleted") && !eventType.EqualsLiteral("modified")) { Here. I think that we should do the same thing. Ignore created. Process modified and deleted.
Attachment #8500332 -
Flags: feedback?(dhylands) → feedback+
Assignee | ||
Comment 7•10 years ago
|
||
Modified the first patch with suggestions in comment 6. Replace "created" event handling with "modified" event handling.
Attachment #8500332 -
Attachment is obsolete: true
Attachment #8501487 -
Flags: review?(dhylands)
Comment 9•10 years ago
|
||
Comment on attachment 8501487 [details] [diff] [review] Bug1074604: Handle file "modified" event in MozMtpDatabase and MozMtpServer Review of attachment 8501487 [details] [diff] [review]: ----------------------------------------------------------------- I don't understand why you're calling RemoveEntry when you get a modified notification. ::: dom/system/gonk/MozMtpDatabase.cpp @@ +271,4 @@ > if (entryHandle != 0) { > + MTP_LOG("About to call sendObjectRemoved Handle 0x%08x file %s", entryHandle, filePath.get()); > + aMtpServer->sendObjectRemoved(entryHandle); > + RemoveEntry(entryHandle); Why are you removing the entry here? ::: dom/system/gonk/MozMtpServer.cpp @@ +108,5 @@ > } > > NS_ConvertUTF16toUTF8 eventType(aData); > + if (!eventType.EqualsLiteral("modified") && !eventType.EqualsLiteral("deleted")) { > + // Bug 1074604: Needn't handle "created" event, once file operation finished, it would trigger "modified" event nit: wrap line at 80 characters
Attachment #8501487 -
Flags: review?(dhylands)
Assignee | ||
Comment 10•10 years ago
|
||
Add comment for removing/readding entry for the existed file
Attachment #8501487 -
Attachment is obsolete: true
Attachment #8501540 -
Flags: review?(dhylands)
Comment 11•10 years ago
|
||
Comment on attachment 8501540 [details] [diff] [review] Bug 1074604: Handle file "modified" event in MozMtpDatabase and MozMtpServer. Review of attachment 8501540 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/MozMtpDatabase.cpp @@ +267,5 @@ > > MtpObjectHandle entryHandle = FindEntryByPath(filePath); > > + if (aEventType.EqualsLiteral("modified")) { > + // To update the file information to the newest, we remove the entry for nit: trailing space @@ +268,5 @@ > MtpObjectHandle entryHandle = FindEntryByPath(filePath); > > + if (aEventType.EqualsLiteral("modified")) { > + // To update the file information to the newest, we remove the entry for > + // the existed file, then re-add the entry for the file. nit: existing @@ +277,4 @@ > } > entryHandle = CreateEntryForFile(filePath, aFile); > if (entryHandle == 0) { > + // creating entry for the file fail, we don't tell MTP nit: s/fail/failed/ s/we// creating entry for the file failed, don't tell MTP ::: dom/system/gonk/MozMtpServer.cpp @@ +108,5 @@ > } > > NS_ConvertUTF16toUTF8 eventType(aData); > + if (!eventType.EqualsLiteral("modified") && !eventType.EqualsLiteral("deleted")) { > + // Bug 1074604: Needn't handle "created" event, once file operation nit: trailing space
Attachment #8501540 -
Flags: review?(dhylands) → review+
Assignee | ||
Comment 12•10 years ago
|
||
Tryserver results fine. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=772478498578
Attachment #8501540 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8501565 [details] [diff] [review] Bug1074604: Handle file "modified" event in MozMtpDatabase and MozMtpServer. r=dhylands Approval Request Comment [Feature/regressing bug #]: It is a bug of MTP implementation. [User impact if declined]: when enable MTP, takes new video cannot appears on the connected host or file would not be completed. [Describe test coverage new/current, TBPL]: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=772478498578 [Risks and why]: (Low risk) It is a correction. [String/UUID change made/needed]: none
Attachment #8501565 -
Flags: approval-mozilla-aurora?
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/abff1c80ccea
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/abff1c80ccea
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S6 (10oct)
Reporter | ||
Updated•10 years ago
|
Component: General → MTP/UMS
Reporter | ||
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage+][COM=Storage] → [QAnalyst-Triage+][COM=MTP/UMS]
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(bbajaj)
Comment 16•10 years ago
|
||
Comment on attachment 8501565 [details] [diff] [review] Bug1074604: Handle file "modified" event in MozMtpDatabase and MozMtpServer. r=dhylands Alison, can you help verify this fix once it lands on 2.1? Thanks!
Attachment #8501565 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(bbajaj)
Updated•10 years ago
|
Comment 17•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fb2c26b10c0b
Reporter | ||
Comment 18•10 years ago
|
||
Verified on [2.1] Gaia-Rev f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1 Gecko-Rev https://hg.mozilla.org/releases/mozilla-aurora/rev/75cd3a8fa031 Build-ID 20141012160202 Version 34.0a2 [2.2] Gaia-Rev 3b81896f04a02697e615fa5390086bd5ecfed84f Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/199fb29c3467 Build-ID 20141012160201 Version 35.0a1
You need to log in
before you can comment on or make changes to this bug.
Description
•