From 63403658411394e8a4d3ab95762a2dd4264ee996 Mon Sep 17 00:00:00 2001
From: Mattias Wadman <mattias.wadman@gmail.com>
Date: Wed, 25 Jan 2023 16:13:56 +0100
Subject: [PATCH] mp4: udta: Handle box with value rest of box

Try distinguish by probing length field. Should probably be improved, what does ffmpeg do?

Regression from c3e3b3e90dd42ab379fa00a74168ce8c41ad8ef5 #553
---
 format/mp4/boxes.go                       | 30 +++++++++++++++++------
 format/mp4/testdata/udta_no_length.fqtest | 12 +++++++++
 2 files changed, 34 insertions(+), 8 deletions(-)
 create mode 100644 format/mp4/testdata/udta_no_length.fqtest

diff --git a/format/mp4/boxes.go b/format/mp4/boxes.go
index c10cebd6..ef4c84fc 100644
--- a/format/mp4/boxes.go
+++ b/format/mp4/boxes.go
@@ -1647,7 +1647,7 @@ func decodeBox(ctx *decodeContext, d *decode.D, typ string) {
 			}
 		})
 	default:
-		// there are at least 3 ways to encode udta metadata in mov/mp4 files.
+		// there are at least 4 ways to encode udta metadata in mov/mp4 files.
 		//
 		// mdta subtype:
 		//
@@ -1656,7 +1656,7 @@ func decodeBox(ctx *decodeContext, d *decode.D, typ string) {
 		//     hdlr with subtype "mdta"
 		//     keys with 1-based <index> to key namespace.name table
 		//     ilst
-		//       <index>-box
+		//       <index>-box (box type is 32bit BE 1-based number into table above)
 		//         data box with value
 		//
 		// mdir subtype:
@@ -1668,20 +1668,34 @@ func decodeBox(ctx *decodeContext, d *decode.D, typ string) {
 		//       ©<abc> or similar
 		//         data with value
 		//
-		// no-meta-box:
+		// no-meta-box with length and language:
 		//
 		// udta
 		//   ©<abc> or similar
-		//     data with length, language and value
-
+		//     value length and language
+		//
+		// no-meta-box value rest of box:
+		//
+		// udta
+		//   <name>
+		//     value rest of box
 		if mb := ctx.currentMetaBox(); mb != nil && ctx.parent().typ == "ilst" {
 			// unknown type under a meta box with ilst as parent, decode as boxes
 			// is probably one or more data boxes
 			decodeBoxes(ctx, d)
 		} else if ctx.parent().typ == "udta" {
-			length := d.FieldU16("length")
-			d.FieldStrFn("language", decodeLang)
-			d.FieldUTF8("value", int(length))
+			// TODO: better probe? ffmpeg uses box name heuristics?
+			// if 16 length field seems to match assume box with length, language and value
+			// otherwise decode as box with value rest of box
+			probeLength := d.PeekBits(16)
+			// +2 for length field, +2 for language field
+			if (probeLength+2+2)*8 == uint64(d.BitsLeft()) {
+				length := d.FieldU16("length")
+				d.FieldStrFn("language", decodeLang)
+				d.FieldUTF8("value", int(length))
+			} else {
+				d.FieldUTF8("value", int(d.BitsLeft()/8))
+			}
 		} else {
 			d.FieldRawLen("data", d.BitsLeft())
 		}
diff --git a/format/mp4/testdata/udta_no_length.fqtest b/format/mp4/testdata/udta_no_length.fqtest
new file mode 100644
index 00000000..0b080584
--- /dev/null
+++ b/format/mp4/testdata/udta_no_length.fqtest
@@ -0,0 +1,12 @@
+# construct udta box with one name box that has no length field
+$ fq -n '[0,0,0,53-32,117,100,116,97,0,0,0,45-32,110,97,109,101,"hello"] | tobytes | mp4({force: true}) | d'
+    |00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f|0123456789abcdef|.{}: (mp4)
+    |                                               |                |  boxes[0:1]:
+    |                                               |                |    [0]{}: box
+0x00|00 00 00 15                                    |....            |      size: 21
+0x00|            75 64 74 61                        |    udta        |      type: "udta" (User-data)
+    |                                               |                |      boxes[0:1]:
+    |                                               |                |        [0]{}: box
+0x00|                        00 00 00 0d            |        ....    |          size: 13
+0x00|                                    6e 61 6d 65|            name|          type: "name"
+0x10|68 65 6c 6c 6f|                                |hello|          |          value: "hello"