[
https://issues.apache.org/jira/browse/TIKA-4850?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110160#comment-18110160
]
ASF GitHub Bot commented on TIKA-4850:
--------------------------------------
Copilot commented on code in PR #3090:
URL: https://github.com/apache/tika/pull/3090#discussion_r3900930813
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java:
##########
@@ -179,16 +181,54 @@ public void testMP4MultipleCovers() throws Exception {
assertEquals(3, metadataList.size());
//a png data atom (well-known type 14) followed by a jpeg one (13)
+ //covr carries no picture type, so the first image is the thumbnail
Metadata front = metadataList.get(1);
assertEquals("image/png", front.get(HttpHeaders.CONTENT_TYPE));
- assertEquals(TikaCoreProperties.EmbeddedResourceType.INLINE.toString(),
+
assertEquals(TikaCoreProperties.EmbeddedResourceType.THUMBNAIL.toString(),
front.get(TikaCoreProperties.EMBEDDED_RESOURCE_TYPE));
Metadata back = metadataList.get(2);
assertEquals("image/jpeg", back.get(HttpHeaders.CONTENT_TYPE));
assertEquals(TikaCoreProperties.EmbeddedResourceType.INLINE.toString(),
back.get(TikaCoreProperties.EMBEDDED_RESOURCE_TYPE));
}
+ /**
+ * A cover in a second udta box is still an inline picture: the first
+ * cover of the file is its thumbnail, not the first of every box
+ */
+ @Test
+ public void testCoversAcrossUserDataBoxes() throws Exception {
+ byte[] file;
+ try (InputStream is =
getResourceAsStream("/test-documents/testMP4_coverArt.m4a")) {
+ file = is.readAllBytes();
+ }
+ //append a copy of the file's udta box (with its covr) at the top level
+ int udta = indexOf(file, "udta".getBytes(StandardCharsets.ISO_8859_1))
- 4;
+ int size = ByteBuffer.wrap(file, udta, 4).getInt();
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
Review Comment:
`indexOf(...)` can return -1 (or match an incidental "udta" byte sequence),
which makes `udta = ... - 4` negative/out of bounds and yields a
hard-to-diagnose exception before the assertions. Add guards that the udta
marker was found at a plausible position and that the declared box size fits
within the file before slicing/copying.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/ogg/OggAudioParser.java:
##########
@@ -117,8 +114,14 @@ protected static void extractChannelInfo(Metadata
metadata, int channelCount) {
}
}
- protected static void extractComments(Metadata metadata,
XHTMLContentHandler xhtml,
- VorbisStyleComments comments, ParseContext context)
+ /**
+ * @return the pictures carried in the comments; the caller emits them
+ * through {@link #extractPictures(List, XHTMLContentHandler,
ParseContext)},
+ * {@link FlacParser} first merges them with the native PICTURE blocks so
+ * one file yields one thumbnail
+ */
+ protected static List<PictureBlock> extractComments(Metadata metadata,
+ XHTMLContentHandler xhtml, VorbisStyleComments comments,
ParseContext context)
throws IOException, TikaException, SAXException {
Review Comment:
`extractComments(...)` no longer uses the `ParseContext context` parameter
after the refactor (pictures are returned and emitted by the caller). Keeping
an unused parameter is misleading and makes future refactors harder; consider
removing it and updating the small set of call sites accordingly.
> Mark audio cover art as THUMBNAIL embedded resources
> ----------------------------------------------------
>
> Key: TIKA-4850
> URL: https://issues.apache.org/jira/browse/TIKA-4850
> Project: Tika
> Issue Type: New Feature
> Reporter: Dominik Schmidt
> Priority: Major
>
> The audio parsers (Mp3Parser, MP4Parser/TikaUserDataBox, OggAudioParser,
> FlacParser) emit embedded pictures with tk:embedded-resource-type=INLINE. For
> a client that wants "the representative image of this file" that is
> inconsistent with the other container parsers: ODF, OOXML, iWork, GeoGebra
> and RawTiffParser all mark that image THUMBNAIL, so a client can pick the
> first THUMBNAIL without knowing the format. For audio it currently needs a
> special case (INLINE image, and for ID3/FLAC preferably the one whose
> dc:description is "Cover (front)").
> INLINE also does not describe what cover art is: it is not a picture placed
> in the document's text flow, it is the picture that stands for the file.
> Proposal:
> - ID3v2 APIC and FLAC/Ogg PICTURE blocks: emit picture type 3 "Cover (front)"
> as THUMBNAIL. If a tag has no front cover, emit the first picture as
> THUMBNAIL. All other pictures stay INLINE (or become ATTACHMENT).
> - MP4 covr: the atom carries no picture type; emit the first cover as
> THUMBNAIL, further ones INLINE.
> - Keep dc:description with the picture type name as it is.
> This changes the resource type of one embedded document per audio file;
> clients filtering on INLINE to find cover art would need to accept THUMBNAIL
> as well. Worth a CHANGES note.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
