Skip to content

Don't misinterpret DJI info maker note as DJI maker note #10470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Don't misinterpret DJI info maker note as DJI maker note #10470

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78c05e2
Fix debug output typos in exif
nielsdos Jan 29, 2023
0051ba4
Don't misinterpret DJI info maker note as DJI maker note
nielsdos Jan 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions ext/exif/exif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,9 @@ typedef struct {
mn_offset_mode_t offset_mode;
} maker_note_type;

/* Some maker notes (e.g. DJI info tag) require custom parsing */
#define REQUIRES_CUSTOM_PARSING NULL

/* Remember to update PHP_MINFO if updated */
static const maker_note_type maker_note_array[] = {
{ tag_table_VND_CANON, "Canon", NULL, 0, 0, MN_ORDER_INTEL, MN_OFFSET_NORMAL},
Expand All @@ -1287,6 +1290,7 @@ static const maker_note_type maker_note_array[] = {
{ tag_table_VND_OLYMPUS, "OLYMPUS OPTICAL CO.,LTD", "OLYMP\x00\x01\x00", 8, 8, MN_ORDER_NORMAL, MN_OFFSET_NORMAL},
{ tag_table_VND_SAMSUNG, "SAMSUNG", NULL, 0, 0, MN_ORDER_NORMAL, MN_OFFSET_NORMAL},
{ tag_table_VND_PANASONIC, "Panasonic", "Panasonic\x00\x00\x00", 12, 12, MN_ORDER_NORMAL, MN_OFFSET_NORMAL},
{ REQUIRES_CUSTOM_PARSING, "DJI", "[ae_dbg_info:", 13, 13, MN_ORDER_MOTOROLA, MN_OFFSET_NORMAL},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This [ae_dbg_info: is where I'm unsure. The exiftools docs lists a couple of DJI Info Tags, and ae_dbg_info is just one of those. Just checking for [ (like in my earlier POC) is likely too brittle.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This [ae_dbg_info: is where I'm unsure. The exiftools docs lists a couple of DJI Info Tags, and ae_dbg_info is just one of those. Just checking for [ (like in my earlier POC) is likely too brittle.

I used https://exiftool.org/makernote_types.html to determine how to identify the info tag. Exiftool itself only checks for "[ae_dbg_info:" (https://github.com/exiftool/exiftool/blob/48df8aae22faa33d830dcf2ecdf406998b4d3849/lib/Image/ExifTool/Exif.pm#L3402) so that's why I thought that might suffice here. According to the makernote_types.html table, it seems like for two cameras it suffices to check for "[ae_dbg_info:".

Also, it might make sense to add a regression test, although that ~9MB picture provided by the bug reporter might be a bit large.

Yeah, I tried to shrink that image but that destroys the metadata in the editor I used. There's also some test images you can find on wikimedia.

{ tag_table_VND_DJI, "DJI", NULL, 0, 0, MN_ORDER_NORMAL, MN_OFFSET_NORMAL},
{ tag_table_VND_SONY, "SONY", "SONY DSC \x00\x00\x00", 12, 12, MN_ORDER_NORMAL, MN_OFFSET_NORMAL},
{ tag_table_VND_SONY, "SONY", NULL, 0, 0, MN_ORDER_NORMAL, MN_OFFSET_NORMAL},
Expand Down Expand Up @@ -3168,10 +3172,16 @@ static bool exif_process_IFD_in_MAKERNOTE(image_info_type *ImageInfo, char * val
return true;
}

if (UNEXPECTED(maker_note->tag_table == REQUIRES_CUSTOM_PARSING)) {
/* Custom parsing required, which is not implemented at this point
* Return true so that other metadata can still be parsed. */
return true;
}

dir_start = value_ptr + maker_note->offset;

#ifdef EXIF_DEBUG
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Process %s @x%04X + 0x%04X=%d: %s", exif_get_sectionname(section_index), (intptr_t)dir_start-(intptr_t)info->offset_base+maker_note->offset+displacement, value_len, value_len, exif_char_dump(value_ptr, value_len, (intptr_t)dir_start-(intptr_t)info->offset_base+maker_note->offset+displacement));
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Process %s @0x%04X + 0x%04X=%d: %s", exif_get_sectionname(section_index), (intptr_t)dir_start-(intptr_t)info->offset_base+maker_note->offset+displacement, value_len, value_len, exif_char_dump(value_ptr, value_len, (intptr_t)dir_start-(intptr_t)info->offset_base+maker_note->offset+displacement));
#endif

ImageInfo->sections_found |= FOUND_MAKERNOTE;
Expand Down Expand Up @@ -3330,7 +3340,7 @@ static bool exif_process_IFD_TAG_impl(image_info_type *ImageInfo, char *dir_entr
#ifdef EXIF_DEBUG
dump_data = exif_dump_data(&dump_free, format, components, ImageInfo->motorola_intel, value_ptr);
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE,
"Process tag(x%04X=%s,@x%04X + x%04X(=%d)): %s%s %s",
"Process tag(x%04X=%s,@0x%04X + x%04X(=%d)): %s%s %s",
tag, exif_get_tagname_debug(tag, tag_table), offset_val+displacement, byte_count, byte_count, (components>1)&&format!=TAG_FMT_UNDEFINED&&format!=TAG_FMT_STRING?"ARRAY OF ":"", exif_get_tagformat(format), dump_data);
if (dump_free) {
efree(dump_data);
Expand Down Expand Up @@ -4173,7 +4183,7 @@ static bool exif_process_IFD_in_TIFF_impl(image_info_type *ImageInfo, size_t dir
}
entry_offset = php_ifd_get32u(dir_entry+8, ImageInfo->motorola_intel);
#ifdef EXIF_DEBUG
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Next IFD: %s @x%04X", exif_get_sectionname(sub_section_index), entry_offset);
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Next IFD: %s @0x%04X", exif_get_sectionname(sub_section_index), entry_offset);
#endif
exif_process_IFD_in_TIFF(ImageInfo, entry_offset, sub_section_index);
if (section_index!=SECTION_THUMBNAIL && entry_tag==TAG_SUB_IFD) {
Expand Down