dschmidt commented on code in PR #2927:
URL: https://github.com/apache/tika/pull/2927#discussion_r3528050515
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageMetadataExtractor.java:
##########
@@ -306,6 +310,53 @@ public void handle(Directory directory, Metadata metadata)
throws MetadataExcept
}
}
+ /**
+ * Copies the XMP properties parsed by Metadata Extractor into the
metadata,
+ * keyed by their {@code prefix:name} path. The other handlers copy a
+ * directory's tags, but XMP keeps its properties in a separate map
+ * ({@link XmpDirectory#getXmpProperties()}), so without this they are
lost.
+ * A property is skipped when its key is already set or matches a known
Tika
+ * field, so normalized values from other handlers are not overwritten.
+ */
+ static class XmpHandler implements DirectoryHandler {
+
+ static {
+ // XMPCore's namespace registry is process-global and keeps the
first
+ // prefix it sees for a URI. Pin canonical prefixes so keys stay
stable
+ // (files use both Camera and GCamera for the Google photo
namespace).
+ // https://developer.android.com/media/platform/motion-photo-format
+ try {
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/camera/";, "Camera");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/";, "Container");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/item/";, "Item");
+ } catch (XMPException e) {
+ // non-fatal: only affects prefix stability
+ }
Review Comment:
Thanks, I went back and forth on this one. These URIs are constant and
valid, so `registerNamespace` can't actually throw here; the catch only
satisfies the checked signature. If registration ever did break, the keys would
become non-deterministic (parse-order dependent), which `MotionPhotoXmpTest`
catches in CI. Rethrowing from a static initializer would surface as
`ExceptionInInitializerError` and break all image parsing, so I'd rather keep
it caught. I've expanded the comment to spell this out.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageMetadataExtractor.java:
##########
@@ -306,6 +310,53 @@ public void handle(Directory directory, Metadata metadata)
throws MetadataExcept
}
}
+ /**
+ * Copies the XMP properties parsed by Metadata Extractor into the
metadata,
+ * keyed by their {@code prefix:name} path. The other handlers copy a
+ * directory's tags, but XMP keeps its properties in a separate map
+ * ({@link XmpDirectory#getXmpProperties()}), so without this they are
lost.
+ * A property is skipped when its key is already set or matches a known
Tika
+ * field, so normalized values from other handlers are not overwritten.
+ */
+ static class XmpHandler implements DirectoryHandler {
+
+ static {
+ // XMPCore's namespace registry is process-global and keeps the
first
+ // prefix it sees for a URI. Pin canonical prefixes so keys stay
stable
+ // (files use both Camera and GCamera for the Google photo
namespace).
+ // https://developer.android.com/media/platform/motion-photo-format
+ try {
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/camera/";, "Camera");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/";, "Container");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/item/";, "Item");
+ } catch (XMPException e) {
+ // non-fatal: only affects prefix stability
+ }
+ }
+
+ public boolean supports(Class<? extends Directory> directoryType) {
+ return XmpDirectory.class.isAssignableFrom(directoryType);
+ }
+
+ public void handle(Directory directory, Metadata metadata) throws
MetadataException {
+ Map<String, String> properties = ((XmpDirectory)
directory).getXmpProperties();
Review Comment:
Left these off to match the surrounding code: none of the other
`DirectoryHandler` implementations in this file use `@Override` on
`supports`/`handle`. Happy to add them across all of them in a separate change
if you'd prefer.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/MotionPhotoXmpTest.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.image;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+import org.xml.sax.helpers.DefaultHandler;
+
+import org.apache.tika.TikaTest;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.parser.ParseContext;
+
+/**
+ * Google Motion Photos keep their metadata in a vendor XMP namespace. Two
+ * variants share that namespace: the current Motion Photo format and the
legacy
+ * MicroVideo format. Both are covered here.
+ */
+public class MotionPhotoXmpTest extends TikaTest {
+
+ /** XMP from a vendor namespace (Google Motion Photo) is exposed, not
dropped. */
+ @Test
+ public void testMotionPhotoXmpIsExposed() throws Exception {
+ Metadata metadata = new Metadata();
+ metadata.set(Metadata.CONTENT_TYPE, "image/jpeg");
+ try (TikaInputStream tis =
+
getResourceAsStream("/test-documents/testJPEG_MotionPhoto.jpg")) {
+ new JpegParser().parse(tis, new DefaultHandler(), metadata, new
ParseContext());
+ }
+
+ assertEquals("1", metadata.get("Camera:MotionPhoto"));
+ assertEquals("1", metadata.get("Camera:MotionPhotoVersion"));
+ assertEquals("500000",
metadata.get("Camera:MotionPhotoPresentationTimestampUs"));
+ // The embedded video item (its byte length lets a client range-fetch
the
+ // video without downloading the whole file) is exposed too.
+ assertEquals("MotionPhoto",
metadata.get("Container:Directory[2]/Item:Semantic"));
+ assertEquals("122562",
metadata.get("Container:Directory[2]/Item:Length"));
+ }
+
+ /** Keys use the canonical prefix even when the file declares another
(GCamera). */
+ @Test
+ public void testCanonicalPrefixIsStable() throws Exception {
+ Metadata metadata = new Metadata();
+ metadata.set(Metadata.CONTENT_TYPE, "image/jpeg");
+ try (TikaInputStream tis =
+
getResourceAsStream("/test-documents/testJPEG_MicroVideo.jpg")) {
+ new JpegParser().parse(tis, new DefaultHandler(), metadata, new
ParseContext());
+ }
+ assertEquals("1", metadata.get("Camera:MicroVideo"));
+ assertEquals("4182318", metadata.get("Camera:MicroVideoOffset"));
+ assertEquals(null, metadata.get("GCamera:MicroVideoOffset"));
Review Comment:
Done, switched to `assertNull`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
