Make WordPress Core

Opened 12 months ago

Last modified 2 months ago

#58240 accepted defect (bug)

wp_read_image_metadata() doesn't handle Exif array values

Reported by: dd32's profile dd32 Owned by: joedolson's profile joedolson
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: needs-patch
Focuses: Cc:

Description

Some images appear to have multiple values for some headers, which causes wp_read_image_metadata() to return 'Array' for some fields.

A real-life example are these images:

Looking at the return value from exif_read_data() for one of these, you'll see arrays below. However wp_read_image_metadata() includes lines like (string) $exif['FocalLength'];. It looks like iPhone 13 mini might be common between the above examples

[...]
  'SectionsFound' =>
  string(19) "ANY_TAG, IFD0, EXIF"
  'COMPUTED' =>
  array(6) {
[...]
    'ApertureFNumber' =>
    string(5) "f/1.6"
  }
[...]
  'Make' =>
  string(5) "Apple"
  'Model' =>
  string(48) "iPhone 13 mini back dual wide camera 5.1mm f/1.6"
  'Exif_IFD_Pointer' =>
  int(106)
  'ApertureValue' =>
  array(2) {
    [0] =>
    string(9) "1356/1000"
    [1] =>
    string(9) "8803/1000"
  }
[...]
  'FocalLength' =>
  array(2) {
    [0] =>
    string(9) "5100/1000"
    [1] =>
    string(10) "10884/1000"
  }
  'ShutterSpeedValue' =>
  array(2) {
    [0] =>
    string(10) "10884/1000"
    [1] =>
    string(19) "1124129791/84149760"
  }

As a result on the above Photo pages you'll see something like this:

Focal Length: Arraymm
Shutter Speed: 1/0

This could be related to the image processing application being used, or the sections found in the image. I'd be tempted to just select the first array key in those cases.

Change History (14)

#1 @dd32
12 months ago

  • Focuses rest-api removed

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


12 months ago

#3 follow-up: @joedolson
12 months ago

  • Milestone changed from Awaiting Review to 6.3
  • Owner set to joedolson
  • Status changed from new to accepted

This could definitely be better. My first thought is that this relates to the multi-camera arrays on the phone, since I see it's citing an iphone 13 with dual cameras. I'm inclined to implode the array and store it as comma separated values, although for these numerics, we should assume that there could be plugins using this data numerically. Might require a bit of research to decide whether it's better to change the data type from a numeric string to a stringy string or omit some of the image data.

#4 in reply to: ↑ 3 @dd32
12 months ago

Replying to joedolson:

I'm inclined to implode the array and store it as comma separated values

That approach seems sane to me, although I question if it could just be stored as a literal array too.. but for compatibility, probably best to store it as a single stringy value.

although for these numerics, we should assume that there could be plugins using this data numerically

That appears to be the case for the Photo Directory at least, although it looks like it's anticipating an array key anyway and imploding those:
https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/plugins/photo-directory/inc/photo.php?marks=903#L841

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


10 months ago

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


9 months ago

#7 @mukesh27
9 months ago

  • Keywords needs-patch added
  • Milestone changed from 6.3 to 6.4

This ticket was discussed during the bug scrub.

No progress since last few week, Moving to 6.4

Additional props: @OGlekler

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


9 months ago

This ticket was mentioned in Slack in #core by marybaum. View the logs.


7 months ago

#10 @marybaum
7 months ago

Speaking for the September 12 bug-scrubbers, I wonder if we could see a patch with the CSV values in a string and test that. That would at least be movement, and if it doesn't break anything, it could land in 6.4!

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


7 months ago

#12 @antpb
7 months ago

  • Milestone changed from 6.4 to 6.5

With RC1 very close @joedolson mentioned being more comfortable moving this to future so we can close the other tasks assigned for 6.4

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


2 months ago

#14 @audrasjb
2 months ago

  • Milestone changed from 6.5 to Future Release

As per today's bug scrub, we decided to move this ticket to Future Release to give it more time to get a patch.

Note: See TracTickets for help on using tickets.