Closed
Bug 815079
Opened 12 years ago
Closed 12 years ago
[BLUETOOTH] Can't see received image in Gallery if the image size is too big or Gallery has been already launched
Categories
(Firefox OS Graveyard :: General, defect, P2)
Tracking
(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)
People
(Reporter: carlosmartinez, Assigned: echou)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
Tested in unagi device with:
Gaia - 56e45e8
Gecko - 4eaacd3
STR:
1-Go to settings
2-Activate bluetooth
3-Search for devices and pair the unagi with the other
4-Send an image from the other device to the unagi
5-Accept the file transfer
Expected result --> You´re able to see the image in the gallery
Actual result --> You cannot find the received image
Reporter | ||
Updated•12 years ago
|
blocking-basecamp: --- → ?
Assignee | ||
Comment 1•12 years ago
|
||
cc David, the owner of Gallery app
Hi Carlos,
Currently the Gallery app does not show the image whose size exceeds 5 MB pixel(*), so it might not be the problem of not receiving the image but cannot be shown.
To confirm, please go to |adb shell| and see if the file exists under folder /sdcard/downloads/bluetooth. Moreover, please choose an image smaller than 5 MB pixel and check if it would be shown correctly.
Thanks in advance.
* Referencing bug (about the limit of image size): Bug 809782
Hi David,
According to your answer to Dominic's question for patch 1 on Bug 809782, current "5 MB pixel limit" seems not the final implementation. Due to that users may send photos which were taken by their DCs or higher level smart phones, I was wondering if we could relax the restriction to 10 MB or more.
Thanks in advance.
Eric
Reporter | ||
Comment 2•12 years ago
|
||
Hi Eric,
You´re right, image is stored in /sdcard/downloads/bluetooth folder but is not shown by gallery app. File is only 2.6 MB so I don´t think the problem is with the size but... with the name? maybe the folder name?
Please, let me know if I have to close this bug and open another one related to Gallery or if we can move this one.
Thanks.
Carlos
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to carlosmartineztoral from comment #2)
> Hi Eric,
>
> You´re right, image is stored in /sdcard/downloads/bluetooth folder but is
> not shown by gallery app. File is only 2.6 MB so I don´t think the problem
> is with the size but... with the name? maybe the folder name?
>
I don't think this is caused by the name. AFAIK, currently Gallery app only shows the image smaller than a specific size, and I'm pretty sure that a 2.6 MB image is too large to be shown.
> Please, let me know if I have to close this bug and open another one related
> to Gallery or if we can move this one.
>
No problem. I have already cc'ed djf, the owner of Gallery app. Still waiting for his response, I think David can confirm if this is a bug or not.
> Thanks.
>
> Carlos
Thank you, Carlos.
Eric
Assignee | ||
Updated•12 years ago
|
Flags: needinfo?(dflanagan)
Comment 4•12 years ago
|
||
To prevent out of memory crashes, I've limited the gallery to ignoring images that have a file size of > 3 megabytes or a resolution of more than 5 megapixels..
I made those numbers up, because they seem like safe values. We might be able to go higher, but I'm not confident that we won't run into memory problems. A 5 megapixel image requires 20mb of ram to decode. And the gallery app is running in an environment where there is often < 80mb of free memory.
Carlos: could you check the pixel resolution of your image? If width*height is more than 5*1024*1024 then that is the issue. (And there will be a message in logcat telling you that gallery ignored the image.)
On the other hand, I've heard other anecdotal reports (but no solid bug filed yet) of gallery not detecting new files right away and needing to be restarted. When this bug occurs, would you try quitting gallery and restarting it? Does the image appear then? If so, that's a clear gallery bug, and you should assign this to me.
Flags: needinfo?(dflanagan)
Comment 5•12 years ago
|
||
Carlos, we need more information does it work with smaller images ?
Reporter | ||
Comment 6•12 years ago
|
||
Hi all,
I´ve tested some more things and here it goes what I´ve found:
- It works with smaller images but I have to kill gallery app if its already opened in order to see the image just received
- The first image I´ve tested with is 8 megapixel but I can´t see any warning message in adb logcat
Regards
Flags: needinfo?(carlos.martinez)
Comment 7•12 years ago
|
||
(In reply to carlosmartineztoral from comment #6)
> Hi all,
>
> I´ve tested some more things and here it goes what I´ve found:
>
> - It works with smaller images but I have to kill gallery app if its already
> opened in order to see the image just received
>
Hmm. I wonder if the bluetooth code is not going through device storage... Any part of the system that writes files must do it through device storage so that running media apps are notified of the new file. Otherwise the apps don't know about the new file until they restart and do a full scan of the sdcard.
I've asked dougt to make sure that the files are being created in such a way that device storage knows about them and can send appropriate events.
> - The first image I´ve tested with is 8 megapixel but I can´t see any
> warning message in adb logcat
Sorry. I thought I had a warning there. You wouldn't see it until the restart, of course. I think for v2 we should consider exposing these warnings to the user somehow, but I suspect it is too late to do that now.
>
> Regards
Comment 8•12 years ago
|
||
If I was going to start working on this bug right now, I'd insert debugging statements into the 'change' event handler in shared/js/mediadb.js to see what sort of events are being received by the gallery when a file is being received via bluetooth. (If the events aren't happening then we have a gecko problem)
Then I'd probably try to continue tracing things into the gallery/js/MetadataParser.js to see if the received file is being correctly handled by gallery. (This is where it would be rejected if it is too big, for example).
And I'd also try adding debuging output in the events ('created', I think) that mediadb sends to the gallery app, in apps/js/gallery.js to see if the gallery is being correctly notified of the new file. If this event isn't being received, then there is a problem in mediadb.
Updated•12 years ago
|
blocking-basecamp: ? → +
Updated•12 years ago
|
Keywords: regression
Comment 9•12 years ago
|
||
Doug: could this be related to #817496?
Assignee | ||
Updated•12 years ago
|
Summary: [BLUETOOTH] Receive images from another device is not working → [BLUETOOTH] Can't see received image in Gallery if the image size is too big or Gallery has been already launched
Updated•12 years ago
|
Assignee: nobody → iliu
Priority: -- → P2
Assignee | ||
Comment 10•12 years ago
|
||
(In reply to David Flanagan [:djf] from comment #7)
> (In reply to carlosmartineztoral from comment #6)
> > Hi all,
> >
> > I´ve tested some more things and here it goes what I´ve found:
> >
> > - It works with smaller images but I have to kill gallery app if its already
> > opened in order to see the image just received
> >
>
> Hmm. I wonder if the bluetooth code is not going through device storage...
> Any part of the system that writes files must do it through device storage
> so that running media apps are notified of the new file. Otherwise the apps
> don't know about the new file until they restart and do a full scan of the
> sdcard.
Currently it didn't go through device storage to save the received file. I
didn't know that gaia apps couldn't get the notification in that case.
Last week I tried to implement receiving and saving file through device storage in Gecko but encountered problems. First, I don't know how to save an unfinished byte stream to a file. Since the user may send a large file to us and split it to many packets based on OBEX protocol, we definitely do not want to keep the whole file in memory, so I was wondering if there is a way to continue writing data to the file. Second, when creating a DeviceStorageFile object, a nsAString "storage type" parameter is needed for its ctor, which means that we have to decide which storage ("MVP", 'music', 'video', or 'pictures') shall be used to save the received file. Not sure if it is right.
Doug, could you give some advice about "how to save received file and ensure launched Gaia apps will be notified"? Thanks in advance.
> I've asked dougt to make sure that the files are being created in such a way
> that device storage knows about them and can send appropriate events.
>
> > - The first image I´ve tested with is 8 megapixel but I can´t see any
> > warning message in adb logcat
>
> Sorry. I thought I had a warning there. You wouldn't see it until the
> restart, of course. I think for v2 we should consider exposing these
> warnings to the user somehow, but I suspect it is too late to do that now.
>
So we won't support viewing large image in Gallery for v1, is that right? Just want to make sure about this, if it's the case, then maybe we can just reject the file transfer request sent from remote when the file length exceeds our limit.
Flags: needinfo?(doug.turner)
Comment 11•12 years ago
|
||
Eric: You're correct. Gallery can't open really big images. An 8megapixel image requires 32 megabytes when decoded, and memory is just too tight. In order to prevent OOM crashes, Gallery protects itself by just ignoring images that are too big.
Currently "too big" is defined by some arbitrary limits I've imposed, and we can probably change those limits, but there will still have to be limits of some kind.
Assignee | ||
Comment 12•12 years ago
|
||
(In reply to David Flanagan [:djf] from comment #11)
> Eric: You're correct. Gallery can't open really big images. An 8megapixel
> image requires 32 megabytes when decoded, and memory is just too tight. In
> order to prevent OOM crashes, Gallery protects itself by just ignoring
> images that are too big.
>
> Currently "too big" is defined by some arbitrary limits I've imposed, and we
> can probably change those limits, but there will still have to be limits of
> some kind.
ok, then I'll discuss with Ian to see if we could do something for users before they see the confirmation prompt. Thank you, David.
Comment 13•12 years ago
|
||
David and Eric,
Do we need a file size checking for the received file before open it(request an activity "open")?
If the size is bigger the limitation, we should show a confirmation prompt for user.
Or could we have the confirmation prompt in the Gallery APP?
Updated•12 years ago
|
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Comment 14•12 years ago
|
||
Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.
Target Milestone: --- → B2G C3 (12dec-1jan)
Assignee | ||
Comment 15•12 years ago
|
||
* This patch will solve 50% of this problem (Can't see received image in Gallery if it has been already launched)
* The solution is similar to Bug 817496
* Because it is possible to receive any kind of files, so I need to check the MIME type of each received file and put them into corresponding DeviceStorage.
Assignee: iliu → echou
Attachment #689671 -
Flags: review?(doug.turner)
Flags: needinfo?(doug.turner)
Comment 16•12 years ago
|
||
Comment on attachment 689671 [details] [diff] [review]
patch 1: v1: notify DeviceStorage that we've just received a file
Review of attachment 689671 [details] [diff] [review]:
-----------------------------------------------------------------
r+ with those changes.
::: dom/bluetooth/BluetoothOppManager.cpp
@@ +475,5 @@
> + } else if (StringBeginsWith(mimeType, NS_LITERAL_CSTRING("video/"))) {
> + mDsFile = new DeviceStorageFile(NS_LITERAL_STRING("movies"), f);
> + } else if (StringBeginsWith(mimeType, NS_LITERAL_CSTRING("audio/"))) {
> + mDsFile = new DeviceStorageFile(NS_LITERAL_STRING("music"), f);
> + }
So, our mime service on b2g isn't great. There is a pretty good chance that you'll find a file that we don't have a good mimetype for. This would mean that mDsFile is going to be null. You may want to assert() if you file a file that doesn't have a mimetype so that you can go add support for it in gecko.
@@ +1121,5 @@
> + } else if (mDsFile) {
> + nsString data;
> + CopyASCIItoUTF16("modified", data);
> + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
> + obs->NotifyObservers(mDsFile, "file-watcher-update", data.get());
In the unlikely event that obs is null, you'll crash here. Better protect against that.
Attachment #689671 -
Flags: review?(doug.turner) → review+
Updated•12 years ago
|
QA Contact: wachen
Assignee | ||
Comment 17•12 years ago
|
||
* Nits picked. Have left a warning which would be shown if we can't get correct mime type for further debugging.
Attachment #689671 -
Attachment is obsolete: true
Assignee | ||
Comment 18•12 years ago
|
||
Assignee | ||
Comment 19•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Component: Gaia::Bluetooth File Transfer → General
QA Contact: wachen
Comment 21•12 years ago
|
||
another test..orz
Component: Gaia::Bluetooth File Transfer → General
QA Contact: wachen
Assignee | ||
Comment 22•12 years ago
|
||
To sum up, this issue (Couldn't see the received image in launched Gallery app) was actually caused by two points:
(1) The image size exceeds the limit.
(2) Bluetooth module didn't notify DeviceStorage after transferring finished, therefore no update would be done in Gallery.
The (1) has been explained by David in comment 4 & comment 11, and my patch fixed (2).
Please let me know if you have any questions. Thank you.
Eric
Comment 23•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 24•12 years ago
|
||
Comment 25•12 years ago
|
||
This is fixed in the platform now, but viewing received photos still isn't working because I broke the Gallery's Open activity a week or two ago and never noticed that it was broken.
I'm fixing it in 822507, and am marking this bug as depending on that one so that I can use the blocking+ here to land my fix!
Depends on: 822507
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•