Closed Bug 815079 Opened 7 years ago Closed 7 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)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)

VERIFIED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
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
blocking-basecamp: --- → ?
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
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
(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
Flags: needinfo?(dflanagan)
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)
Flags: needinfo?(carlos.martinez)
Keywords: smoketest
Carlos, we need more information does it work with smaller images ?
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)
(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
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.
blocking-basecamp: ? → +
Doug: could this be related to #817496?
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
Assignee: nobody → iliu
Priority: -- → P2
(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)
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.
(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.
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?
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → ARM
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)
* 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 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+
QA Contact: wachen
* 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
Component: Gaia::Bluetooth File Transfer → General
QA Contact: wachen
test
Component: General → Gaia::Bluetooth File Transfer
QA Contact: wachen
another test..orz
Component: Gaia::Bluetooth File Transfer → General
QA Contact: wachen
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
https://hg.mozilla.org/mozilla-central/rev/f0951ef3bc4d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.