Add WebP support to ext/exif#22213
Conversation
|
|
||
| static bool exif_scan_WEBP_header(image_info_type *ImageInfo, size_t riff_size) | ||
| { | ||
| static const uchar ExifHeader[] = {0x45, 0x78, 0x69, 0x66, 0x00, 0x00}; |
There was a problem hiding this comment.
Could you add a commment explaining this? As looking from the spec it's not immediately obvious why this is the corresponding Exif header? Or is this just {'E', 'X', 'I', 'F', 0, 0}? In which chase writting it that way would be more obvious. Not everyone knows ASCII by heart.
There was a problem hiding this comment.
Added a comment. It's the Exif\0\0 identifier code, the same array as in exif_process_APP1, so I kept the hex form to match it.
There was a problem hiding this comment.
Alternatively, I can update both places to admittedly more clear {'E', 'X', 'I', 'F', 0, 0}, just doesn't make sense to do the same thing in 2 different ways (imho)
There was a problem hiding this comment.
I think it makes sense to do so in a follow-up PR to update both places.
WebP stores EXIF metadata in a RIFF "EXIF" chunk whose payload is a raw TIFF block, the same data JPEG carries after its APP1 marker. Walk the RIFF chunks, locate the EXIF chunk, and hand its payload to the existing exif_process_TIFF_in_JPEG() parser, so no third-party library is needed. The chunk is accepted regardless of the VP8X extended-format flag, and an optional leading "Exif\0\0" marker is skipped when an encoder emits one. Closes phpGH-19904
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
Minor nit but LGTM.
Other thing that might be good to refactor is renaming the motorale_intel stuff to isLittleEndian or something to make it a bit clearer.
| char *data; | ||
| size_t skip = 0; | ||
| bool ret = false; | ||
|
|
||
| if (chunk_size < 8) { | ||
| return false; | ||
| } | ||
|
|
||
| data = emalloc(chunk_size); |
There was a problem hiding this comment.
nit:
| char *data; | |
| size_t skip = 0; | |
| bool ret = false; | |
| if (chunk_size < 8) { | |
| return false; | |
| } | |
| data = emalloc(chunk_size); | |
| size_t skip = 0; | |
| bool ret = false; | |
| if (chunk_size < 8) { | |
| return false; | |
| } | |
| char *data = emalloc(chunk_size); |
2 participants
WebP keeps EXIF metadata in a RIFF
EXIFchunk whose payload is a raw TIFF block, the same bytes a JPEG carries after itsAPP1/Exif\0\0marker, soexif_read_data()can read it by handing the chunk to the existingexif_process_TIFF_in_JPEG()parser, with no new dependency. Chunk scanning is bounded by the declared RIFF size, theEXIFchunk is accepted whether or not the VP8X extended-format flag is set, and an optional leadingExif\0\0marker is tolerated. The phpinfo supported-filetypes line now lists what exif actually parses (it was missing HEIF too).Exif Spec: https://developers.google.com/speed/webp/docs/riff_container
Fixes #19904