Skip to content
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.

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

GTiff: save XMP on field TIFFTAG_XMLPACKET #767

Merged
merged 2 commits into from
Jul 26, 2018

Conversation

jjimenezshaw
Copy link
Contributor

@jjimenezshaw jjimenezshaw commented Jul 18, 2018

What does this PR do?

This PR saves the XMP xml data in the field TIFFTAG_XMLPACKET when GTiffDataset::SetMetadata is called with the domain xml:XMP.

The method GTiffDataset::OpenOffset reads the field TIFFTAG_XMLPACKET and sets the value on the metadata domain xml:XMP. That gives the user access to the XMP xml string (and read/parse it on their own).

However, there is not way to write the XMP data in the field TIFFTAG_XMLPACKET. Third party software (like Exiv2) is not able then to read the XMP from a tiff file created with GDAL.

What are related issues/pull requests?

None

Tasklist

  • Add test case(s)
  • Review
  • Adjust for comments
  • All CI builds and checks have passed

Copy link
Member

@rouault rouault left a comment

Choose a reason for hiding this comment

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

Could you add a new test case in autotest/gcore/tiff_write.py testing this ?

@@ -18244,6 +18244,12 @@ CPLErr GTiffDataset::SetMetadata( char ** papszMD, const char *pszDomain )
}
}

if( EQUAL(pszDomain, "xml:XMP") && papszMD && *papszMD )
Copy link
Member

Choose a reason for hiding this comment

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

pszDomain != nullptr should be tested

@@ -18244,6 +18244,12 @@ CPLErr GTiffDataset::SetMetadata( char ** papszMD, const char *pszDomain )
}
}

if( EQUAL(pszDomain, "xml:XMP") && papszMD && *papszMD )
{
int nTagSize = strlen(*papszMD) + 1;
Copy link
Member

Choose a reason for hiding this comment

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

explicit static_cast should be done.
Are you sure that the nul terminated byte must be written ? I didn't check the specification, but the autotest/gdrivers/data/byte_with_xmp.tif doesn't include a nul terminated byte for example

@jjimenezshaw
Copy link
Contributor Author

About the test case, I don't see how can I test it.
The problem is that the metadata is stored in the GDALMetadata as well. So if I set the xml:XMP metadata, save the file, open it again (with GDAL) and read the metadata, it will be there anyway (even without my PR). The difference on this PR is that it is stored in the TIFFTAG_XMLPACKET tiff field.

I do not see how can I access directly to this field in python, not using "GetMetadata". Is there any option?

@rouault
Copy link
Member

rouault commented Jul 21, 2018

The problem is that the metadata is stored in the GDALMetadata as well. So if I set the xml:XMP metadata, save the file, open it again (with GDAL) and read the metadata, it will be there anyway (even without my PR)

OK, so that should also be fixed so that it is written only once in TIFFTAG_XMLPACKET and not in the GDAL TIFF tag.

@rouault rouault merged commit ded6003 into OSGeo:master Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants