dschmidt commented on PR #3526:
URL: https://github.com/apache/tika/pull/3526#issuecomment-6660787306
My agent says:
- `SubtitleTracks.wanted()` (SubtitleTracks.java:93-101) asks
`shouldParseEmbedded` about a phantom `track-0.vtt` before any track is found,
and `checkEmbeddedLimits` sets the limit flags as a side effect: an MP4/MKV
without subtitles at the depth or count limit now reports
`EMBEDDED_DEPTH_LIMIT_REACHED`/`EMBEDDED_RESOURCE_LIMIT_REACHED` (e.g.
maxEmbeddedDepth=1, zip holding an .mkv), and with `throwOnMax*` the exception
is swallowed. Probing only once a text track exists would avoid the false flags.
- MKV: a SimpleBlock, or a BlockGroup without BlockDuration, becomes a
zero-length cue `T --> T` (MkvSubtitleExtractor.java:352, asserted in
MkvSubtitleExtractorTest.java:136). WebVTT wants end > start; the track's
DefaultDuration (not read today) or the next block's timestamp could supply it.
- SRT tracks lose every `{...}` (MkvSubtitleExtractor.java:79, 415), so
`Press {Enter}` loses `{Enter}`; `SubtitleParser` only strips `{\...}` override
blocks.
- The 64 MB cap also applies to content already in memory
(MP4Parser.java:207-213): a byte[]-backed MP4 over 64 MB gets no subtitles,
while the docs say "or a source that can hold it in memory".
- Mp4SubtitleExtractor.java:404-409 warns on every text/sbtl/subt track that
isn't tx3g/text, so CMAF files with `wvtt`/`stpp` tracks now carry a container
warning they didn't before. Intended?
- `extractSubtitles` only takes effect from `tika-config.json`; a
per-request `mp4-parser`/`matroska-parser` block is accepted and ignored (same
as `maxBoxSize`).
- Unverified: BOM-less QuickTime `text` samples are decoded as UTF-8
(Mp4SubtitleExtractor.java:563), so older Mac-encoded text would come out as
U+FFFD.
Tests:
- The `tryRetainInMemory()` branch (MP4Parser.java:212, re-openable
`TikaInputStream.get(IOSupplier, ...)`) has no test; forcing it to false fails
nothing.
- `testFileInputOverSpoolLimit` writes its 100 MB sparse file with
`Files.createTempFile` into the system temp dir; `@TempDir` is the convention
here.
- `testCorruptedBytes` (both classes) only reports the seed on a
RuntimeException, so a `@Timeout` hang or an Error loses it. The `catch
(IOException)` in the MP4 one is dead, the extractor records IOExceptions
itself.
Nits: SubtitleTracks.java:79 says a Matroska stream "is read to its end",
but the walk stops at the first Cluster without a text track; CHANGES.txt:73 is
whitespace-only; the PR title got truncated ("…embedded …") and would become
the squash subject.
--
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]
