dschmidt commented on PR #3114: URL: https://github.com/apache/tika/pull/3114#issuecomment-5501558113
Honestly, having sat with this for a bit: I am not sure any more that it is worth it. I built it for #3115, where the video trailing a motion photo has to be typed before it is emitted. It turns out not to buy anything there, because the embedded parse runs `MP4Parser`, which sets the type correctly anyway. That is the general picture, too. `MP4Parser` already does what this detector does: `isAudioOnly` marks an audio-only file as `audio/mp4`, and the compatible brand list gets `testMP4Video.mp4` to `video/mp4`, which `MP4ParserTest` asserts today. Both files attached to TIKA-3646 are already typed correctly after a parse. What is left over is detect-only callers, and I do not have a real use case for that beyond tidiness. So folding it into the parser would mostly mean writing what the parser already does. As I see it: 1. Close this, and answer TIKA-3646 with "the parse gets it right, detection alone does not". 2. Keep the detector, but opt-in only (`spi = false`, the way `Pkcs7Detector` is), for callers that need the type without a parse. 3. Keep only the `Mp4Boxes` extraction, which shares the box primitives instead of leaving a third copy of a box walk around, and drop the detector. I lean towards 1, or 3 if the shared primitives are worth keeping on their own. Your call. -- 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]
