[ 
https://issues.apache.org/jira/browse/TIKA-4838?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18107550#comment-18107550
 ] 

ASF GitHub Bot commented on TIKA-4838:
--------------------------------------

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.





> Expose the MP4 audio and video track codecs (audio:format, video:format)
> ------------------------------------------------------------------------
>
>                 Key: TIKA-4838
>                 URL: https://issues.apache.org/jira/browse/TIKA-4838
>             Project: Tika
>          Issue Type: New Feature
>            Reporter: Dominik Schmidt
>            Priority: Major
>
> The MP4/QuickTime parser does not expose the per-track codec identifiers. 
> TikaMp4SoundHandler and TikaMp4VideoHandler read the sample-entry format 
> fourcc but only use it for other purposes (audio: DRM detection; video: 
> nothing).
> Add audio:format and video:format, each carrying the four-character codec 
> identifier from the track's sample description (audio mp4a/alac/ac-3, video 
> avc1/hev1). For protected streams (drms/enca, encv/drmi) the original codec 
> is recovered from the nested frma box.
> The existing xmpDM:audioCompressor only fires for audio-typed files and 
> carries the container major brand, and xmpDM:videoCompressor carries the 
> compressor name (encoder string, e.g. "Lavc… libx264"), so neither gives the 
> per-track codec fourcc for video/* files.
> Rounds out the per-track metadata added in TIKA-4779, TIKA-4800 and TIKA-4802.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to