tballison commented on PR #3062:
URL: https://github.com/apache/tika/pull/3062#issuecomment-5431655063
From my claude:
```
Worth acting on
1. Protected entries report the wrapper fourcc, not the codec
(TikaMp4SoundHandler.java:94, same in Video). For drms/enca/encv/drmi, ISO
14496-12 puts the real codec in
the child sinf/frma box; the handler already walks child boxes for
esds/btrt, so frma is reachable. As-is a FairPlay .m4p gives audio:format=drms
— and audio:has-drm
already carries that fact. Once released, the value semantics are
frozen. (The testMP4_drm.m4a fixture's drms entry is a bare stub with no sinf,
so a real sample is
needed to test.)
2. Last-entry/last-track wins, undocumented (TikaMp4SoundHandler.java:94).
FORMAT is overwritten per stsd entry and per track, HAS_DRM is set on any
entry. [enca, mp4a]
→ has-drm=true, format=mp4a; AAC+E-AC-3 tracks → format=ec-3 while
bitrate comes from the AAC track. Pick first-entry or document it; no
multi-entry test guards it.
3. fourcc emitted verbatim with no printability check
(TikaMp4VideoHandler.java:108). Previously only compared; now a public metadata
value. Corrupt bytes 00 01 02 03
land as control chars in XML output. Gate on printable ASCII 0x20–0x7E
and trim QuickTime trailing spaces (raw , rle ).
4 -- I don't agree with
Design question — decide before release
5. Generic audio:format/video:format in tika-core get MP4-only fourcc
semantics; no other A/V parser populates them, and audio:encoding +
xmpDM:audioCompressor/videoCompressor already express codec identity in
other vocabularies. Either name it for what it is (e.g. mp4-prefixed /
"sample-entry fourcc") or
define it container-neutrally with MP4 as first producer. Precedent cuts
both ways (audio:has-drm is already MP4-only).
6. New javadoc in Audio.java:14 / Video.java:35 calls
xmpDM:audioCompressor "the human-readable codec name" — but MP4Parser sets it
from the ftyp brand (M4A, which the
test asserts) and videoCompressor from the raw encoder string. Reword or
drop the cross-ref.
Minor
7. Sound and Video handlers now have byte-identical stsd walk loops +
private fourCc(); the Sound copy already diverged (pos + 18 <= end guard).
Neither handles size==1
(largesize) or size==0. Worth a shared walker.
8. Comment churn: 3-line justify-to-reviewer test comment in
MP4ParserTest.java:133; enca comment lost its "ISO common encryption" anchor
(relevant to #1).
```
Let me know what you think.
--
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]
