[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFP: mapillary-tools -- Useful tools and scripts related to Mapillary



[cc:ing debian-python in case anyone happens to know enough about python3-contruct to provide some clues]

OK, so it turns out that there are problems packaging pymp4.

It depends on construct, a (nice) library for parsing binary
formats. However said library seems to have little interest in
maintaining any sort of stable API so there have been significant
changes between 2.8, 2.9 and 2.10 (and in fact pymp4 needs 2.8.8 quite
specifically, and doesn't even work with 2.8.16).

2.8.8 is from October 2016 and Debian now has 2.10.x in stable, testing and unstable. 

There is a bug in construct 2.8.8 which means that pymp4 fails 5 out of 30-odd tests even with that.
A python class moved, so that is trivially fixed with:

--- construct-2.8.8.orig/construct/core.py
+++ construct-2.8.8/construct/core.py
@@ -997,7 +997,7 @@ class Range(Subconstruct):
	 max = self.max(context) if callable(self.max) else self.max
	 if not 0 <= min <= max <= sys.maxsize:
	     raise RangeError("unsane min %s and max %s" % (min, max))
-        if not isinstance(obj, collections.Sequence):
+        if not isinstance(obj, collections.abc.Sequence):
	     raise RangeError("expected sequence type, found %s" % type(obj))
	 if not min <= len(obj) <= max:
	     raise RangeError("expected from %d to %d elements, found %d" % (min, max, len(obj)))


But as no-one cares about 2.8.x anyway this fix doesn't help much.

What's really needed is updating pymp4 to use construct 2.10 (or
switch to a more stable library if such a thing exists ('Kaitai
Struct' was mentioned)).

There is an upstream issue for this:
https://github.com/beardypig/pymp4/issues/3

Which I've just added some info to.

2.9 changes the way Strings work: an encoding is always required, and
explicit flavours of padding (left/right, specifiable padding char)
have been removed. pymp4 uses these padding options (specifying spaces
and right-padding), but may still work fine with the remaining default
behaviour of 'PaddedString' (nulls and rightpading). I don't know
enough about either the MP4 format or the codebase to be sure. I did
know up a patch to update to the new string API.

2.9 also loses Embedded bitwise structs. And 2.10 loses 'Embedded' completely.

I have not really managed to work out exactly what 'Embedded' does. I
can't really work out what the difference between putting a bitwise
struct in the middle of a struct and putting an Embedded bitwise
struct in the middle of a struct is. Mostly this is because the online
docs only cover 2.10, not 2.8 so I don't know what the old definition
was. I spent a couple of hours trying to work it out. It's made more
complicated by the fact that construct also switched from 'bits by
default' to 'bytes by default' for efficiency reasons.

I've put a half-arsed patch in the github issue which is probably OK
for the strings part and almost certainly wrong for the Embedded
part. So example I have no idea how to deal with this which selects
one struct depending on a string:
Box = PrefixedIncludingSize(Int32ub, Struct(
    "offset" / TellMinusSizeOf(Int32ub),
    "type" / Peek(String(4, padchar=b" ", paddir="right")),
    Embedded(Switch(this.type, {
        b"ftyp": FileTypeBox,
        b"styp": SegmentTypeBox,
        b"mvhd": MovieHeaderBox,
        b"moov": ContainerBoxLazy,
	...
	b'afrt': HDSFragmentRunBox
    }, default=RawBox)),
    "end" / Tell
))

changing
-    "type" / Peek(String(4, padchar=b" ", paddir="right")), 
to
+    "type" / Peek(PaddedString(4,"ascii")),
is probably equivalent, but what is the equivalent syntax for choosing
the right struct for the 'type' field according to the 1st 4 bytes of
it, without using 'Embedded'?  This was where I decided it was bedtime
and admitted defeat for the time being.

If someone familiar with construct 2.8 to 2.10 upgrades wanted to take
a look at this that would be very helpful. For some things we might
need to know something about the mp4 format too. I'm not sure to what
degree we need to understand the format, or if we can more or less
mechanically update the syntax.

So, for now there is no mapillary-tools in Debian, and without a
response from upstream or some help I'm stuck.

Wookey
--
Principal hats:  Linaro, Debian, Wookware, ARM
http://wookware.org/

Attachment: signature.asc
Description: PGP signature


Reply to: