Repository: beardypig/pymp4 (language: python)

Issue:
Definition of tenc box seems invalid or too leniant
I've attached a sample `tenc` box from https://sho.com, specifically the show Yellowjackets S02E05 on the 2160p track.

![image](https://user-images.githubusercontent.com/17136956/233861866-8042fd19-2094-44e3-819c-dc47a88829d8.png)

It fails to parse with the following error:

```
Traceback (most recent call last):
  File "C:\Program Files\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Program Files\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Program Files\Python310\Scripts\mp4dump.exe\__main__.py", line 7, in <module>
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\pymp4\cli.py", line 26, in dump
    box = Box.parse_stream(fd)
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\construct\core.py", line 186, in parse_stream
    return self._parse(stream, context, "parsing")
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\pymp4\parser.py", line 46, in _parse
    obj = self.subcon._parse(stream2, context, path)
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\construct\core.py", line 855, in _parse
    subobj = list(sc._parse(stream, context, path).items())
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\construct\core.py", line 297, in _parse
    return self.subcon._parse(stream, context, path)
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\construct\core.py", line 1544, in _parse
    obj = self.cases.get(key, self.default)._parse(stream, context, path)
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\construct\core.py", line 859, in _parse
    subobj = sc._parse(stream, context, path)
  File "C:\Users\User\AppData\Roaming\Python\Python310\site-packages\construct\core.py", line 2700, in _parse
    raise e.__class__("%s\n    %s" % (e, path))
construct.core.ConstError: expected 0 but parsed 25
    parsing -> _reserved1
```

It's failing at the byte 0xD with the value 0x19 (25 decimal). This offset would be the `_reserved1` definition.

[tenc_box_from_showtime_exact.zip](https://github.com/beardypig/pymp4/files/11304733/tenc_box_from_showtime_exact.zip)

Majority of tests fail on Python 3.10 and newer
See the following pytest warning and summary information:

```
tests/test_box.py::BoxTests::test_ftyp_build
  /home/runner/.cache/pypoetry/virtualenvs/pymp4-_EUa4su2-py3.7/lib/python3.7/site-packages/construct/core.py:1000: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3,and in 3.9 it will stop working
    if not isinstance(obj, collections.Sequence):
```

```
FAILED tests/test_box.py::BoxTests::test_ftyp_build - AttributeError: module 'collections' has no attribute 'Sequence'
FAILED tests/test_box.py::BoxTests::test_mdhd_build - AttributeError: module 'collections' has no attribute 'Sequence'
FAILED tests/test_box.py::BoxTests::test_mdhd_parse - ValueError: closing stream but 1 unwritten bytes remain, 8 is encoded unit
FAILED tests/test_box.py::BoxTests::test_moov_build - AttributeError: module 'collections' has no attribute 'Sequence'
```

All of these except one are failing because Python 3.9+ had `Sequence` move from `collections` to `collections.abc`, i.e.:

```diff
-from collections import Sequence
+from collections.abc import Sequence
```

Sequence is never used within pymp4's code, but it is used heavily by construct. This indicates that construct 2.8.8 does not have true support for 3.9+ and upgrading to a newer version of construct is likely required.

It's not immediately clear how we will proceed from here as I'm not sure when construct resolved this internally, and it is unsure if the update that does fix it needs any more fixes.

I would recommend looking into upgrading construct to 2.10.x. Updating to the newest 2.8.x release (2.8.22) doesn't seem to resolve this issue (after doing other changes, see https://github.com/devine-dl/pymp4/tree/construct-2.8.22-patch

A reminder that there is already a pull request by another kind person, #9, which seems to do a lot of the work for upgrading to 2.10.x

EDIT: Confusingly it seems support for Python 3.9 is there, and the warning states it's dropping support in Python 3.10. As of the current master, tests up as far as Python 3.9 do in fact succeed. Not sure where the difference is here. Either way, tests on Python 3.10+ support are definitely broken.


Discussion / hints:

Fix patch:
diff --git a/src/pymp4/parser.py b/src/pymp4/parser.py
index 1c2311c..bfe2b50 100644
--- a/src/pymp4/parser.py
+++ b/src/pymp4/parser.py
@@ -694,18 +694,26 @@ def _encode(self, obj, context):
 
 TrackEncryptionBox = Struct(
     "type" / If(this._.type != b"uuid", Const(b"tenc")),
-    "version" / Default(Int8ub, 0),
+    "version" / Default(OneOf(Int8ub, (0, 1)), 0),
     "flags" / Default(Int24ub, 0),
-    "_reserved0" / Const(Int8ub, 0),
-    "_reserved1" / Const(Int8ub, 0),
-    "is_encrypted" / Int8ub,
-    "iv_size" / Int8ub,
+    "_reserved" / Const(Int8ub, 0),
+    "default_byte_blocks" / Default(IfThenElse(
+        this.version > 0,
+        BitStruct(
+            # count of encrypted blocks in the protection pattern, where each block is 16-bytes
+            "crypt" / Nibble,
+            # count of unencrypted blocks in the protection pattern
+            "skip" / Nibble
+        ),
+        Const(Int8ub, 0)
+    ), 0),
+    "is_encrypted" / OneOf(Int8ub, (0, 1)),
+    "iv_size" / OneOf(Int8ub, (0, 8, 16)),
     "key_ID" / UUIDBytes(Bytes(16)),
-    "constant_iv" / Default(If(this.is_encrypted and this.iv_size == 0,
-                               PrefixedArray(Int8ub, Byte),
-                               ),
-                            None)
-
+    "constant_iv" / Default(If(
+        this.is_encrypted and this.iv_size == 0,
+        PrefixedArray(Int8ub, Byte)
+    ), None)
 )
 
 SampleEncryptionBox = Struct(


Test patch:
diff --git a/tests/test_dashboxes.py b/tests/test_dashboxes.py
index e1b014b..3500619 100644
--- a/tests/test_dashboxes.py
+++ b/tests/test_dashboxes.py
@@ -32,6 +32,8 @@ def test_tenc_parse(self):
             (type=b"tenc")
             (version=0)
             (flags=0)
+            (_reserved=0)
+            (default_byte_blocks=0)
             (is_encrypted=1)
             (iv_size=8)
             (key_ID=UUID('337b9643-21b6-4355-9e59-3eccb46c7ef7'))