[wp-trac] [WordPress Trac] #28077: Use exif_imagetype instead of getimagesize in file_is_displayable_image

WordPress Trac noreply at wordpress.org
Sat May 3 20:42:35 UTC 2014


#28077: Use exif_imagetype instead of getimagesize in file_is_displayable_image
-------------------------+------------------------------
 Reporter:  joehoyle     |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Awaiting Review
Component:  Media        |     Version:  3.9
 Severity:  normal       |  Resolution:
 Keywords:               |     Focuses:
-------------------------+------------------------------

Comment (by joehoyle):

 Replying to [comment:2 jdgrimes]:
 > (I believe there are some places in core where error suppression is used
 only if `WP_DEBUG` is false, so maybe `getimagesize()` should be
 suppressed only conditionally here as well. ?)

 Yup I'd agree with that.

 > You need to check if is_callable( 'exif_imagetype' ) and fall back to
 getimagesize() if not, because the exif extension may not be loaded. That
 may negate the speed increases.

 Ahh, good point - perhaps a static variable to reduce repetition of the
 check? Though I'd imagine `is_callable()` maybe internally caches that.

 I finally tracked out why `getimagesize` fails on a Stream, it's if the
 Stream is not `seekable`, as `getimagesize` needs to read a much larger
 chunk of the file - hence why `exif_imagetype` is a lot faster I would
 presume.

 There is probably another 5+ calls to `getimagesize` that could be
 migrated to `exif_imagetype`, so I'd only be interested in doing that  if
 it's going to be "worth" it. At this point it's seeming like a micro
 optimisation that may not be necessary.

 Cons:
 - Needs to check if `exif_imagetype` and fallback anyway
 - Slightly higher code-complexity for little gains

 Pros:
 - Makes plugins with non seekable streams (AWS SDK for S3 for example)
 work
 - Slightly less disk / network IO
 - Slightly faster

 I'd be happy if a more experience core contributor decides this is not
 worth and closes this ticket :)

--
Ticket URL: <https://core.trac.wordpress.org/ticket/28077#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list