You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tika.apache.org by "dschmidt (via GitHub)" <gi...@apache.org> on 2026/08/24 13:05:10 UTC
[PR] TIKA-4838: Expose the MP4 audio track codec (audio:format) [tika]
dschmidt opened a new pull request, #3062:
URL: https://github.com/apache/tika/pull/3062
Adds `audio:format`, the audio track's four-character codec identifier from the MP4/QuickTime sample description (e.g. `mp4a` for AAC, `alac`, `ac-3`).
`TikaMp4SoundHandler` already reads the sample-entry format fourcc (it uses it to detect `drms`/`enca` protected streams); this exposes it as metadata. For protected streams the original codec is recovered from the nested `frma` box, with `audio:has-drm` still set.
`xmpDM:audioCompressor` only fires for audio-typed files and carries the container brand, not the per-track codec, so it doesn't cover `video/*` files.
Verified against the M4A test fixture (`mp4a`); metadata schema registries updated.
https://issues.apache.org/jira/browse/TIKA-4838
Follows TIKA-4779, TIKA-4800 and TIKA-4802.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs as FourCCs (audio:fourcc, video:fourcc) [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on PR #3062:
URL: https://github.com/apache/tika/pull/3062#issuecomment-5432058044
Thanks, all addressed:
1. Protected entries now report the original codec from `sinf`/`frma`; a bare entry without `sinf` still reports the protected format. Since there was no real sample, the fixtures are synthetic, built per ISO/IEC 14496-12 8.12 and cross-checked against FFmpeg's and GPAC's writers: a FairPlay `drms` (esds + sinf/frma mp4a) and testMP4Video.mp4 with its `avc1` turned into `encv` (sinf before btrt).
2. Kept last-entry-wins, consistent with `audio:bitrate`/`video:bitrate`; it's documented in the javadoc and now guarded by a two-entry fixture (mp4a then ac-3).
3. FourCCs are only exposed when all bytes are printable ASCII, with QuickTime's trailing padding trimmed (`raw `, `rle `).
5. Renamed to `audio:fourcc` / `video:fourcc`. FourCCs aren't MP4-specific (QuickTime, AVI, ...), so no container prefix, but the name now says what the value is. A human-readable codec name would need the esds objectTypeIndication and a mapping table, so I'd keep that for a follow-up.
6. Dropped the cross-reference to `xmpDM:audioCompressor`/`videoCompressor`.
7. Both handlers share `Mp4SampleEntries` for the stsd walk, which now also handles size 0 and largesize entries (unit-tested).
8. Trimmed the test comment, restored the ISO common encryption anchor.
Not sure which item 4 was, happy to look if you paste it.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs as FourCCs (audio:fourcc, video:fourcc) [tika]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3062:
URL: https://github.com/apache/tika/pull/3062#discussion_r3868648912
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/Mp4SampleEntries.java:
##########
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.mp4;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.tika.io.EndianUtils;
+
+/**
+ * Walks the sample entries of a SampleDescriptionBox ('stsd') payload, shared
+ * by the sound and video handlers. The payload is 4 bytes version and flags,
+ * a 4 byte entry count, then one sample entry per count. Each entry is a box:
+ * a 32-bit size and a FourCC, where a size of 1 announces a 64-bit largesize
+ * and a size of 0 extends the entry to the end of the payload (ISO/IEC
+ * 14496-12, 4.2).
+ */
+final class Mp4SampleEntries {
+
+ /**
+ * Size of the SampleEntry fields that follow the box header in every
+ * entry: 6 reserved bytes and the 2 byte data reference index.
+ */
+ static final int SAMPLE_ENTRY_FIELDS = 8;
+
+ interface Visitor {
+ /**
+ * @param fourCC the entry's FourCC, or null if it is not printable
+ * @param b the stsd payload
+ * @param start offset of the first byte after the entry's box header
+ * @param end offset one past the entry's last byte
+ */
+ void entry(String fourCC, byte[] b, int start, int end);
+ }
+
+ private Mp4SampleEntries() {
+ }
+
+ static void walk(byte[] b, Visitor visitor) {
+ if (b.length < 8) {
+ return;
+ }
+ long entryCount = EndianUtils.getUIntBE(b, 4);
+ int pos = 8;
+ for (long i = 0; i < entryCount && pos + 8 <= b.length; i++) {
+ long size = EndianUtils.getUIntBE(b, pos);
+ int header = 8;
+ if (size == 1) {
+ //largesize: the 64-bit size follows the FourCC
+ if (pos + 16 > b.length) {
+ return;
+ }
+ //a value beyond 63 bits goes negative and fails the size check below
+ size = (EndianUtils.getUIntBE(b, pos + 8) << 32) | EndianUtils.getUIntBE(b, pos + 12);
+ header = 16;
+ } else if (size == 0) {
+ //the entry extends to the end of the box
+ size = b.length - pos;
+ }
+ if (size < header + SAMPLE_ENTRY_FIELDS || size > b.length - pos) {
+ return;
+ }
+ int end = pos + (int) size;
+ visitor.entry(printableFourCC(b, pos + 4), b, pos + header, end);
+ pos = end;
+ }
+ }
+
+ /**
+ * Returns whether a sample entry FourCC marks a protected stream: 'drms'
+ * and 'drmi' (FairPlay) or 'enca' and 'encv' (ISO common encryption).
+ */
+ static boolean isProtected(String fourCC) {
+ return "drms".equals(fourCC) || "enca".equals(fourCC)
+ || "encv".equals(fourCC) || "drmi".equals(fourCC);
+ }
+
+ /**
+ * Looks up the original (unprotected) format of a protected sample entry:
+ * the ProtectionSchemeInfoBox 'sinf' among the entry's child boxes carries
+ * an OriginalFormatBox 'frma' whose payload is the codec FourCC that the
+ * protected format replaced (ISO/IEC 14496-12, 8.12). Returns null if
+ * there is none or it is not printable.
+ *
+ * @param pos offset of the entry's first child box
+ * @param end offset one past the entry's last byte
+ */
+ static String originalFormat(byte[] b, int pos, int end) {
+ int sinf = findBox(b, pos, end, "sinf");
+ if (sinf < 0) {
+ return null;
+ }
+ int frma = findBox(b, sinf + 8, boxEnd(b, sinf, end), "frma");
+ if (frma < 0 || frma + 12 > end) {
+ return null;
+ }
+ return printableFourCC(b, frma + 8);
+ }
Review Comment:
`originalFormat()` validates `frma + 12 <= end` (end of the *entry*), but it doesn’t validate that the `frma` box itself is at least 12 bytes (8-byte header + 4-byte payload). A crafted `frma` with size=8 can pass the current check and cause `printableFourCC(b, frma + 8)` to read 4 bytes from the following box, potentially exposing an incorrect codec FourCC.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs as FourCCs (audio:fourcc, video:fourcc) [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on code in PR #3062:
URL: https://github.com/apache/tika/pull/3062#discussion_r3869568459
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/Mp4SampleEntries.java:
##########
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.mp4;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.tika.io.EndianUtils;
+
+/**
+ * Walks the sample entries of a SampleDescriptionBox ('stsd') payload, shared
+ * by the sound and video handlers. The payload is 4 bytes version and flags,
+ * a 4 byte entry count, then one sample entry per count. Each entry is a box:
+ * a 32-bit size and a FourCC, where a size of 1 announces a 64-bit largesize
+ * and a size of 0 extends the entry to the end of the payload (ISO/IEC
+ * 14496-12, 4.2).
+ */
+final class Mp4SampleEntries {
+
+ /**
+ * Size of the SampleEntry fields that follow the box header in every
+ * entry: 6 reserved bytes and the 2 byte data reference index.
+ */
+ static final int SAMPLE_ENTRY_FIELDS = 8;
+
+ interface Visitor {
+ /**
+ * @param fourCC the entry's FourCC, or null if it is not printable
+ * @param b the stsd payload
+ * @param start offset of the first byte after the entry's box header
+ * @param end offset one past the entry's last byte
+ */
+ void entry(String fourCC, byte[] b, int start, int end);
+ }
+
+ private Mp4SampleEntries() {
+ }
+
+ static void walk(byte[] b, Visitor visitor) {
+ if (b.length < 8) {
+ return;
+ }
+ long entryCount = EndianUtils.getUIntBE(b, 4);
+ int pos = 8;
+ for (long i = 0; i < entryCount && pos + 8 <= b.length; i++) {
+ long size = EndianUtils.getUIntBE(b, pos);
+ int header = 8;
+ if (size == 1) {
+ //largesize: the 64-bit size follows the FourCC
+ if (pos + 16 > b.length) {
+ return;
+ }
+ //a value beyond 63 bits goes negative and fails the size check below
+ size = (EndianUtils.getUIntBE(b, pos + 8) << 32) | EndianUtils.getUIntBE(b, pos + 12);
+ header = 16;
+ } else if (size == 0) {
+ //the entry extends to the end of the box
+ size = b.length - pos;
+ }
+ if (size < header + SAMPLE_ENTRY_FIELDS || size > b.length - pos) {
+ return;
+ }
+ int end = pos + (int) size;
+ visitor.entry(printableFourCC(b, pos + 4), b, pos + header, end);
+ pos = end;
+ }
+ }
+
+ /**
+ * Returns whether a sample entry FourCC marks a protected stream: 'drms'
+ * and 'drmi' (FairPlay) or 'enca' and 'encv' (ISO common encryption).
+ */
+ static boolean isProtected(String fourCC) {
+ return "drms".equals(fourCC) || "enca".equals(fourCC)
+ || "encv".equals(fourCC) || "drmi".equals(fourCC);
+ }
+
+ /**
+ * Looks up the original (unprotected) format of a protected sample entry:
+ * the ProtectionSchemeInfoBox 'sinf' among the entry's child boxes carries
+ * an OriginalFormatBox 'frma' whose payload is the codec FourCC that the
+ * protected format replaced (ISO/IEC 14496-12, 8.12). Returns null if
+ * there is none or it is not printable.
+ *
+ * @param pos offset of the entry's first child box
+ * @param end offset one past the entry's last byte
+ */
+ static String originalFormat(byte[] b, int pos, int end) {
+ int sinf = findBox(b, pos, end, "sinf");
+ if (sinf < 0) {
+ return null;
+ }
+ int frma = findBox(b, sinf + 8, boxEnd(b, sinf, end), "frma");
+ if (frma < 0 || frma + 12 > end) {
+ return null;
+ }
+ return printableFourCC(b, frma + 8);
+ }
Review Comment:
Good catch, fixed: the frma box end is now checked against frma + 12, with a header-only frma regression test in Mp4SampleEntriesTest.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs (audio:format, video:format) [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on code in PR #3062:
URL: https://github.com/apache/tika/pull/3062#discussion_r3847345878
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java:
##########
@@ -352,10 +365,12 @@ private Set<String> getVals(Metadata m, String k) {
@Test
public void testDrmProtectedM4a() throws Exception {
- //the sample description declares a protected 'drms' sample entry
+ //the sample description declares a protected 'drms' sample entry: the
+ //format is the protection scheme and has-drm is set
Review Comment:
Fixed in 706ee4cc21.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs (audio:format, video:format) [tika]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3062:
URL: https://github.com/apache/tika/pull/3062#discussion_r3846007738
##########
tika-core/src/main/java/org/apache/tika/metadata/Audio.java:
##########
@@ -84,6 +84,15 @@ public interface Audio {
*/
Property BITS_PER_SAMPLE = Property.internalInteger("audio:bits-per-sample");
+ /**
+ * The audio track's four-character codec identifier from the MP4/QuickTime
+ * sample description (e.g. "mp4a" for AAC, "alac", "ac-3"), or the protection
+ * scheme ("drms"/"enca") for protected streams, where {@link #HAS_DRM} is also
+ * set. A per-stream value: with several audio tracks it reflects the last one.
+ * Distinct from {@link XMPDM#AUDIO_COMPRESSOR}, the human-readable codec name.
Review Comment:
In this Javadoc, “drms”/“enca” are described as a “protection scheme”, but elsewhere (e.g., HAS_DRM) they’re treated as protected sample entry formats. To avoid confusion with the actual protection scheme type carried in a `schm` box (e.g., `cenc`), consider clarifying the terminology here.
##########
tika-core/src/main/java/org/apache/tika/metadata/Video.java:
##########
@@ -37,4 +37,13 @@ public interface Video {
* reflects the last one.
*/
Property BITRATE = Property.internalInteger("video:bitrate");
+
+ /**
+ * The video track's four-character codec identifier from the MP4/QuickTime
+ * sample description (e.g. "avc1" for H.264, "hvc1"/"hev1" for HEVC), or the
+ * protection scheme ("encv"/"drmi") for protected streams. A per-stream value:
+ * with several video tracks it reflects the last one. Distinct from
Review Comment:
This Javadoc calls “encv”/“drmi” a “protection scheme”, but the value being exposed is the sample-entry type fourcc. Using “protected sample entry format” here would be more precise and avoids conflating it with the scheme type found elsewhere in the MP4 protection boxes.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs (audio:format, video:format) [tika]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3062:
URL: https://github.com/apache/tika/pull/3062#discussion_r3845053050
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4VideoHandler.java:
##########
@@ -76,6 +76,16 @@ private void extractFromSampleDescriptions(byte[] b) {
break;
}
int end = pos + (int) size;
+ String format = fourCc(b, pos + 4);
+ //protected streams replace the codec fourcc with 'encv' (ISO common
+ //encryption) or 'drmi' (FairPlay); the original codec is in the nested frma box
+ if ("encv".equals(format) || "drmi".equals(format)) {
+ String original = findOriginalFormat(b, pos + VISUAL_ENTRY_SIZE, end, 0);
+ if (original != null) {
+ format = original;
+ }
+ }
+ tikaMetadata.set(Video.FORMAT, format);
Review Comment:
New behavior is introduced for protected streams (`encv`/`drmi` → recover original codec from nested `frma`), but the added tests only cover unprotected samples (`avc1`/`mp4a`). Please add a test fixture with protected video and/or audio (e.g., `encv` + `frma=avc1`, and `enca`/`drms` + `frma=mp4a`) and assert that `video:format`/`audio:format` reflect the original codec, and that `audio:has-drm` remains set for protected audio.
##########
tika-core/src/main/java/org/apache/tika/metadata/Video.java:
##########
@@ -37,4 +37,12 @@ public interface Video {
* reflects the last one.
*/
Property BITRATE = Property.internalInteger("video:bitrate");
+
+ /**
+ * The video track's four-character codec identifier from the MP4/QuickTime
+ * sample description (e.g. "avc1" for H.264, "hvc1"/"hev1" for HEVC). For
+ * protected streams this is the original codec from the 'frma' box. Distinct
+ * from {@link XMPDM#VIDEO_COMPRESSOR}, the human-readable compressor name.
Review Comment:
`Video.FORMAT` is populated from iterating sample descriptions; because it uses `Metadata#set(...)` with a SIMPLE cardinality property, later sample entries will overwrite earlier ones. Please document the effective behavior (e.g., 'if multiple sample entries exist, the value reflects the last encountered entry') or adjust extraction to set only once (e.g., first non-null) if that’s the intended contract.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4VideoHandler.java:
##########
@@ -105,6 +115,38 @@ private static int findBtrtAverageBitRate(byte[] b, int pos, int end) {
return 0;
}
+ private static final int MAX_BOX_DEPTH = 10;
+
+ /**
+ * Scans the child boxes of a protected ('encv'/'drmi') sample entry for the
+ * 'frma' (original format) box nested in the 'sinf' protection scheme
+ * information box, and returns its codec fourcc, or null if there is none.
+ */
+ private static String findOriginalFormat(byte[] b, int pos, int end, int depth) {
+ if (depth > MAX_BOX_DEPTH) {
+ return null;
+ }
+ while (pos + 8 <= end) {
+ long size = EndianUtils.getUIntBE(b, pos);
+ if (size < 8 || size > end - pos) {
+ return null;
+ }
+ int boxEnd = pos + (int) size;
+ String type = fourCc(b, pos + 4);
+ if ("frma".equals(type) && pos + 12 <= boxEnd) {
+ return fourCc(b, pos + 8);
+ }
+ if ("sinf".equals(type)) {
+ String original = findOriginalFormat(b, pos + 8, boxEnd, depth + 1);
+ if (original != null) {
+ return original;
+ }
+ }
+ pos = boxEnd;
+ }
+ return null;
+ }
Review Comment:
`findOriginalFormat` logic is duplicated in both `TikaMp4VideoHandler` and `TikaMp4SoundHandler` with only docstring differences. Consider extracting this into a shared helper (e.g., an MP4 box parsing utility used by both handlers) so fixes (bounds/recursion behavior, box traversal rules, etc.) don't need to be applied twice.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs (audio:format, video:format) [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on code in PR #3062:
URL: https://github.com/apache/tika/pull/3062#discussion_r3846531666
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4VideoHandler.java:
##########
@@ -76,6 +76,16 @@ private void extractFromSampleDescriptions(byte[] b) {
break;
}
int end = pos + (int) size;
+ String format = fourCc(b, pos + 4);
+ //protected streams replace the codec fourcc with 'encv' (ISO common
+ //encryption) or 'drmi' (FairPlay); the original codec is in the nested frma box
+ if ("encv".equals(format) || "drmi".equals(format)) {
+ String original = findOriginalFormat(b, pos + VISUAL_ENTRY_SIZE, end, 0);
+ if (original != null) {
+ format = original;
+ }
+ }
+ tikaMetadata.set(Video.FORMAT, format);
Review Comment:
Stale: this reviewed an earlier revision. The frma-recovery path was removed from the PR; protected streams now just report the entry fourcc, and the protected-audio behavior that remains is covered by testDrmProtectedM4a.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4VideoHandler.java:
##########
@@ -105,6 +115,38 @@ private static int findBtrtAverageBitRate(byte[] b, int pos, int end) {
return 0;
}
+ private static final int MAX_BOX_DEPTH = 10;
+
+ /**
+ * Scans the child boxes of a protected ('encv'/'drmi') sample entry for the
+ * 'frma' (original format) box nested in the 'sinf' protection scheme
+ * information box, and returns its codec fourcc, or null if there is none.
+ */
+ private static String findOriginalFormat(byte[] b, int pos, int end, int depth) {
+ if (depth > MAX_BOX_DEPTH) {
+ return null;
+ }
+ while (pos + 8 <= end) {
+ long size = EndianUtils.getUIntBE(b, pos);
+ if (size < 8 || size > end - pos) {
+ return null;
+ }
+ int boxEnd = pos + (int) size;
+ String type = fourCc(b, pos + 4);
+ if ("frma".equals(type) && pos + 12 <= boxEnd) {
+ return fourCc(b, pos + 8);
+ }
+ if ("sinf".equals(type)) {
+ String original = findOriginalFormat(b, pos + 8, boxEnd, depth + 1);
+ if (original != null) {
+ return original;
+ }
+ }
+ pos = boxEnd;
+ }
+ return null;
+ }
Review Comment:
Stale: findOriginalFormat was removed along with the frma-recovery feature, so there is nothing left to extract.
##########
tika-core/src/main/java/org/apache/tika/metadata/Video.java:
##########
@@ -37,4 +37,12 @@ public interface Video {
* reflects the last one.
*/
Property BITRATE = Property.internalInteger("video:bitrate");
+
+ /**
+ * The video track's four-character codec identifier from the MP4/QuickTime
+ * sample description (e.g. "avc1" for H.264, "hvc1"/"hev1" for HEVC). For
+ * protected streams this is the original codec from the 'frma' box. Distinct
+ * from {@link XMPDM#VIDEO_COMPRESSOR}, the human-readable compressor name.
Review Comment:
The last-wins behavior is already documented in the javadoc and matches the existing per-stream convention (e.g. Video.BITRATE).
##########
tika-core/src/main/java/org/apache/tika/metadata/Audio.java:
##########
@@ -84,6 +84,15 @@ public interface Audio {
*/
Property BITS_PER_SAMPLE = Property.internalInteger("audio:bits-per-sample");
+ /**
+ * The audio track's four-character codec identifier from the MP4/QuickTime
+ * sample description (e.g. "mp4a" for AAC, "alac", "ac-3"), or the protection
+ * scheme ("drms"/"enca") for protected streams, where {@link #HAS_DRM} is also
+ * set. A per-stream value: with several audio tracks it reflects the last one.
+ * Distinct from {@link XMPDM#AUDIO_COMPRESSOR}, the human-readable codec name.
Review Comment:
Good catch, fixed in 9ec25dcca2 (javadoc and the handler comments now say protected sample entry format).
##########
tika-core/src/main/java/org/apache/tika/metadata/Video.java:
##########
@@ -37,4 +37,13 @@ public interface Video {
* reflects the last one.
*/
Property BITRATE = Property.internalInteger("video:bitrate");
+
+ /**
+ * The video track's four-character codec identifier from the MP4/QuickTime
+ * sample description (e.g. "avc1" for H.264, "hvc1"/"hev1" for HEVC), or the
+ * protection scheme ("encv"/"drmi") for protected streams. A per-stream value:
+ * with several video tracks it reflects the last one. Distinct from
Review Comment:
Fixed in 9ec25dcca2, same wording change as on the audio side.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs as FourCCs (audio:fourcc, video:fourcc) [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison merged PR #3062:
URL: https://github.com/apache/tika/pull/3062
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs (audio:format, video:format) [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4838: Expose the MP4 audio and video track codecs (audio:format, video:format) [tika]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3062:
URL: https://github.com/apache/tika/pull/3062#discussion_r3846569195
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java:
##########
@@ -352,10 +365,12 @@ private Set<String> getVals(Metadata m, String k) {
@Test
public void testDrmProtectedM4a() throws Exception {
- //the sample description declares a protected 'drms' sample entry
+ //the sample description declares a protected 'drms' sample entry: the
+ //format is the protection scheme and has-drm is set
Review Comment:
Comment says “format is the protection scheme”, but the value being asserted ("drms") is the protected sample-entry type fourcc (the protection scheme type is elsewhere, e.g., in `schm`). This wording is misleading and contradicts the clarified terminology used elsewhere in this PR.
--
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: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org