836109 - [gallery] jpeg metadata parser failing for some jpeg images

Status: RESOLVED FIXED

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

Description

[David Flanagan [:djf]]

One of our partners is testing gallery scanning performance with a bunch of JPEG files of mixed pedigree.  Some of those files cause shared/js/media/jpeg_metadata_parser.js to fail with an exception.  This puts the gallery metadata parser onto a slower path than it might otherwise be.

So fixing this problem may improve startup performance.

Comment 1

[David Flanagan [:djf]]

many of the files that fail include an IFD whose next value is 0x02010002, this is being interpreted as an offset of 33619970 but given the nice round hex numbers it probably means something else.  Some old version of EXIF or JFIF or something?

Comment 2

[David Flanagan [:djf]]

I also see one offset of 0x00EC0000, one of 0x00079000, one of 0xDBFF0000

My guess now is that whatever the EXIF tag is before this I'm parsing it wrong and am off by a byte or two.

Comment 3

[David Flanagan [:djf]]

The errors seem to occur after an EXIF tag of 0 or after an IFD with 0 entries.

Comment 4

[David Flanagan [:djf]]

The errors seem to occur after an EXIF tag of 0 or after an IFD with 0 entries.

http://www.exifviewer.org/ is reporting the same errors that I'm seeing, so it may just be bad files.  I should modify my exif parser to ignore the bad stuff more gracefully.

Comment 5

[David Flanagan [:djf]]

Attachment: link to patch on github

This pull request fixes error messages that were appearing in the console when parsing JPEG files (from our partners) with malformed EXIF data. And it also speeds up jpeg metadata parsing by skipping some EXIF tags and just drilling down directly to the embedded preview image, which is the only EXIF metadata we need.

James, I'm asking you to review this because it grew out of my performance work on gallery.  I don't think this patch will make much difference on our partner's data set because most of those image files do not have an embedded preview image.

I'll do some tests on a large set of images from our own camera to see how much difference this patch makes in scan time.  If it is a noticeable difference, I'll nominate this as a FFOS_perf bug.  Otherwise, I'll just land this fix on master.

Comment 6

[David Flanagan [:djf]]

Scanning 1000 photos with the recent mediadb optimization and without this patch takes
172168 ms.

The same test with this patch takes: 170847.

If those measurements are stable, then this is only shaving 2ms off of the parsing time for each image.

Comment 7

[James Lal [:lightsofapollo]]

Comment on attachment 708139 [details]
link to patch on github

r=me sanity check done only though...

I assume you tested this and know about the parsing (more then me anyway).

Comment 8

[David Flanagan [:djf]]

merged into gaia master: 
https://github.com/mozilla-b2g/gaia/commit/10c533f1f3dce0af9f89998f9dd13fd60c9928d2