[
https://issues.apache.org/jira/browse/TIKA-4850?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109953#comment-18109953
]
ASF GitHub Bot commented on TIKA-4850:
--------------------------------------
tballison commented on PR #3090:
URL: https://github.com/apache/tika/pull/3090#issuecomment-5480927659
From my agent:
```
1. MEDIUM — native FLAC can emit two THUMBNAILs (reached by 2 reviewers,
confirmed by me)
FlacParser.java:104 runs OggAudioParser.extractComments (which emits
metadata_block_picture comment
pictures with its own thumbnailIndex), then :110 runs
extractNativePictures with a second independent pick.
A .flac with one PICTURE block plus one metadata_block_picture comment →
two THUMBNAILs, breaking the PR's
own one-per-file invariant. Fix: have the comment path return
List<PictureBlock>, concatenate with native
blocks in FlacParser, call extractPictures(List) once. Needs a test.
2. LOW — the "unknown type" tier is effectively dead; docs/tests promise
it (3 reviewers)
CoverArt.java:68 uses <= OTHER, contract "negative = unknown". No caller
normalizes: ID3 types are & 0xFF
(ID3v2Frame.java:399,439) so never negative; FLAC only goes negative at ≥
2^31. [back(4), type 200] → back
cover wins, while CHANGES/javadoc/CoverArtTest say the unknown one should.
Cheap fix: callers pass -1 when
type >= ID3Tags.PICTURE_TYPES.length (they already compute that predicate
at Mp3Parser.java:341 /
OggAudioParser.java:275), add (4, 200) → 1 to CoverArtTest.
3. LOW — stale javadoc on two of three sites — Mp3Parser.java:312-313,
OggAudioParser.java:246-247 still
say "front cover (or the first picture, if there is none)"; predates the
"Other" tier (aa2476486e). Point
at {@link CoverArt#thumbnailIndex} instead of restating.
4. LOW — no ID3 parser-level test pins pass-through/order. All three MP3
fixtures are front-first, so a
reversed or all-zero type list still passes. Mp3ParserTest already builds
synthetic ID3v2 tags in-test
(:473, :504); a back-then-front APIC pair asserting INLINE/THUMBNAIL costs
~15 lines, no binary.
5. LOW — FLAC return→break is load-bearing and untested. A future revert
to return would silently drop
already-collected pictures (regression vs 4.0.0). One
truncated-second-block test on a copy of
testFLAC_twoCovers.flac would catch it — but confirm vorbis-java's
FlacNativeFile tolerates the bad length
first.
6. LOW — duplicated emit loop. Mp3Parser.java:317-357 and
OggAudioParser.java:251-285 are the same 25 lines
and already diverge (null-vs-empty checks on mime/description). A
CoverArt.Picture record + one
CoverArt.extractPictures(List<Picture>, …) removes the boxed List<Integer>
dance too; MP4 stays separate.
resourceType(coverCount.getAndIncrement(), 0) reads as magic; inline the
ternary.
```
Let me know what you think. Thank you for your iterations on this.
> 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)
