[gallery] jpeg metadata parser failing for some jpeg images

RESOLVED FIXED

Status

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: djf, Assigned: djf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → dflanagan
(Assignee)

Comment 1

6 years ago
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?
(Assignee)

Comment 2

6 years ago
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.
(Assignee)

Comment 3

6 years ago
The errors seem to occur after an EXIF tag of 0 or after an IFD with 0 entries.
(Assignee)

Comment 4

6 years ago
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.
(Assignee)

Comment 5

6 years ago
Created attachment 708139 [details]
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.
Attachment #708139 - Flags: review?(jlal)
(Assignee)

Comment 6

6 years ago
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.
(Assignee)

Updated

6 years ago
Blocks: 817113
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).
Attachment #708139 - Flags: review?(jlal) → review+
(Assignee)

Comment 8

6 years ago
merged into gaia master: 
https://github.com/mozilla-b2g/gaia/commit/10c533f1f3dce0af9f89998f9dd13fd60c9928d2
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.