Repository: Eyevinn/mp4ff
Pull request #355 (2024-05-24 13:20:53)

Improved url box parsing
The URL box should have a zero-terminated location string, or no data.
This PR supports that the termination zero byte is missing and that there is a URL also when the content
has the flag set to indicate that it is self-contained.

Solves #353.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e42ff04..b8f1805 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,22 +11,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - minimum Go version 1.16.
 - ioutil imports replaced by io and os imports
+- Info (mp4ff-info) output for esds boxes
+- API of descriptors
+- Parsing and info output for url boxes
 
 ### Fixed
 
 - support for parsing of hierarchical sidx boxes
 - handling of partially bad descriptors
-
-### Changed
-
-- Info (mp4ff-info) output for esds boxes
-- API of descriptors
+- handle url boxes missing mandatory zero-ending byte
 
 ### Added
 
 - support for ssix box
 - support for leva box
-- details for descriptors as Info outout
+- details of descriptors as Info outout (mp4ff-info)
 
 ## [0.44.0] - 2024-04-19
 
diff --git a/bits/fixedslicereader.go b/bits/fixedslicereader.go
index e84b1cb..723b307 100644
--- a/bits/fixedslicereader.go
+++ b/bits/fixedslicereader.go
@@ -160,7 +160,7 @@ func (s *FixedSliceReader) ReadFixedLengthString(n int) string {
 }
 
 // ReadZeroTerminatedString - read string until zero byte but at most maxLen
-// Set err and return empty string if no zero byte found
+// Set err and return empty string if no zero byte found.
 func (s *FixedSliceReader) ReadZeroTerminatedString(maxLen int) string {
 	if s.err != nil {
 		return ""
@@ -182,6 +182,29 @@ func (s *FixedSliceReader) ReadZeroTerminatedString(maxLen int) string {
 	}
 }
 
+// ReadPossiblyZeroTerminatedString - read string until zero byte but at most maxLen.
+// If maxLen is reached and no zero-byte, return string and ok = false
+func (s *FixedSliceReader) ReadPossiblyZeroTerminatedString(maxLen int) (str string, ok bool) {
+	startPos := s.pos
+	maxPos := startPos + maxLen
+	for {
+		if s.pos == maxPos {
+			return string(s.slice[startPos:s.pos]), true
+		}
+		if s.pos > maxPos {
+			s.err = errors.New("did not find terminating zero")
+			return "", false
+		}
+		c := s.slice[s.pos]
+		if c == 0 {
+			str = string(s.slice[startPos:s.pos])
+			s.pos++ // Next position to read
+			return str, true
+		}
+		s.pos++
+	}
+}
+
 // ReadBytes - read a slice of n bytes
 // Return empty slice if n bytes not available
 func (s *FixedSliceReader) ReadBytes(n int) []byte {
diff --git a/bits/slicereader.go b/bits/slicereader.go
index ca034c2..61ceefd 100644
--- a/bits/slicereader.go
+++ b/bits/slicereader.go
@@ -19,6 +19,7 @@ type SliceReader interface {
 	ReadInt64() int64
 	ReadFixedLengthString(n int) string
 	ReadZeroTerminatedString(maxLen int) string
+	ReadPossiblyZeroTerminatedString(maxLen int) (str string, ok bool)
 	ReadBytes(n int) []byte
 	RemainingBytes() []byte
 	NrRemainingBytes() int
diff --git a/mp4/descriptors.go b/mp4/descriptors.go
index ffbdca5..01205b9 100644
--- a/mp4/descriptors.go
+++ b/mp4/descriptors.go
@@ -213,6 +213,7 @@ func (e *ESDescriptor) Type() string {
 	return TagType(e.Tag())
 }
 
+// Size is size of payload after tag and size field
 func (e *ESDescriptor) Size() uint64 {
 	var size uint64 = 2 + 1
 	streamDependenceFlag := e.FlagsAndPriority >> 7
@@ -238,6 +239,7 @@ func (e *ESDescriptor) Size() uint64 {
 	return size
 }
 
+// SizeSize is size of size field.
 func (e *ESDescriptor) SizeSize() uint64 {
 	return 1 + uint64(e.sizeFieldSizeMinus1) + 1 + e.Size()
 }
@@ -324,6 +326,19 @@ func (e *ESDescriptor) Info(w io.Writer, specificLevels, indent, indentStep stri
 	return bd.err
 }
 
+// DecoderConfigDescriptor is defined in ISO/IEC 14496-1 Section 7.2.6.6.1
+//
+//	class DecoderConfigDescriptor extends BaseDescriptor : bit(8) tag=DecoderConfigDescrTag {
+//	  bit(8) objectTypeIndication;
+//	  bit(6) streamType;
+//	  bit(1) upStream;
+//	  const bit(1) reserved=1;
+//	  bit(24) bufferSizeDB;
+//	  bit(32) maxBitrate;
+//	  bit(32) avgBitrate;
+//	  DecoderSpecificInfo decSpecificInfo[0 .. 1];
+//	  profileLevelIndicationIndexDescriptor profileLevelIndicationIndexDescr [0..255];
+//	}
 type DecoderConfigDescriptor struct {
 	ObjectType          byte
 	StreamType          byte
@@ -333,7 +348,7 @@ type DecoderConfigDescriptor struct {
 	AvgBitrate          uint32
 	DecSpecificInfo     *DecSpecificInfoDescriptor
 	OtherDescriptors    []Descriptor
-	UnknownData         []byte
+	UnknownData         []byte // Data, probably erronous, that we don't understand
 }
 
 func exceedsMaxNrBytes(sizeFieldSizeMinus1 byte, size uint64, maxNrBytes int) bool {
@@ -464,6 +479,10 @@ func (d *DecoderConfigDescriptor) Info(w io.Writer, specificLevels, indent, inde
 	return bd.err
 }
 
+// DecSpecificInfoDescriptor is a generic DecoderSpecificInfoDescriptor.
+//
+// The meaning of the MPEG-4 audio descriptor is defined in  ISO/IEC 14496-3 Section 1.6.2.1.
+
 type DecSpecificInfoDescriptor struct {
 	sizeFieldSizeMinus1 byte
 	DecConfig           []byte
diff --git a/mp4/url.go b/mp4/url.go
index 1330931..e85dd2a 100644
--- a/mp4/url.go
+++ b/mp4/url.go
@@ -10,9 +10,11 @@ import (
 //
 // Contained in : DrefBox (dref
 type URLBox struct {
-	Version  byte
-	Flags    uint32
-	Location string // Zero-terminated string
+	Version           byte
+	Flags             uint32
+	Location          string // Zero-terminated string
+	NoLocation        bool
+	NoZeroTermination bool
 }
 
 const dataIsSelfContainedFlag = 0x000001
@@ -32,15 +34,20 @@ func DecodeURLBoxSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, e
 	versionAndFlags := sr.ReadUint32()
 	version := byte(versionAndFlags >> 24)
 	flags := versionAndFlags & flagsMask
-	location := ""
-	if flags != dataIsSelfContainedFlag {
-		location = sr.ReadZeroTerminatedString(hdr.payloadLen() - 4)
-	}
-
+	maxLen := hdr.payloadLen() - 4
 	b := URLBox{
-		Version:  version,
-		Flags:    flags,
-		Location: location,
+		Version:    version,
+		Flags:      flags,
+		NoLocation: true,
+	}
+	if maxLen > 0 {
+		b.NoLocation = false
+		var ok bool
+		b.Location, ok = sr.ReadPossiblyZeroTerminatedString(maxLen)
+		b.NoZeroTermination = !ok
+		if len(b.Location) == int(maxLen) {
+			b.NoZeroTermination = true
+		}
 	}
 	return &b, sr.AccError()
 }
@@ -48,9 +55,11 @@ func DecodeURLBoxSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, e
 // CreateURLBox - Create a self-referencing URL box
 func CreateURLBox() *URLBox {
 	return &URLBox{
-		Version:  0,
-		Flags:    dataIsSelfContainedFlag,
-		Location: "",
+		Version:           0,
+		Flags:             dataIsSelfContainedFlag,
+		Location:          "",
+		NoLocation:        true,
+		NoZeroTermination: false,
 	}
 }
 
@@ -62,8 +71,11 @@ func (b *URLBox) Type() string {
 // Size - return calculated size
 func (b *URLBox) Size() uint64 {
 	size := uint64(boxHeaderSize + 4)
-	if b.Flags != uint32(dataIsSelfContainedFlag) {
+	if !b.NoLocation {
 		size += uint64(len(b.Location) + 1)
+		if b.NoZeroTermination {
+			size--
+		}
 	}
 	return size
 }
@@ -87,15 +99,21 @@ func (b *URLBox) EncodeSW(sw bits.SliceWriter) error {
 	}
 	versionAndFlags := (uint32(b.Version) << 24) + b.Flags
 	sw.WriteUint32(versionAndFlags)
-	if b.Flags != dataIsSelfContainedFlag {
-		sw.WriteString(b.Location, true)
+	if !b.NoLocation {
+		sw.WriteString(b.Location, !b.NoZeroTermination)
 	}
 	return sw.AccError()
 }
 
 // Info - write specific box information
 func (b *URLBox) Info(w io.Writer, specificBoxLevels, indent, indentStep string) error {
-	bd := newInfoDumper(w, indent, b, -1, 0)
+	bd := newInfoDumper(w, indent, b, int(b.Version), b.Flags)
+	if b.NoLocation {
+		return bd.err
+	}
 	bd.write(" - location: %q", b.Location)
+	if b.NoZeroTermination {
+		bd.write(" - Warning: no zero termination")
+	}
 	return bd.err
 }


Test patch:
diff --git a/bits/fixedslicereader_test.go b/bits/fixedslicereader_test.go
index c07fec3..5fd977b 100644
--- a/bits/fixedslicereader_test.go
+++ b/bits/fixedslicereader_test.go
@@ -237,6 +237,7 @@ func TestFixedSliceReader(t *testing.T) {
 }
 
 func verifyAccErrorInt(t *testing.T, sr *bits.FixedSliceReader, val int) {
+	t.Helper()
 	if sr.AccError() == nil {
 		t.Errorf("should have had an accumulated error")
 	}
@@ -246,6 +247,7 @@ func verifyAccErrorInt(t *testing.T, sr *bits.FixedSliceReader, val int) {
 }
 
 func verifyAccErrorString(t *testing.T, sr *bits.FixedSliceReader, val string) {
+	t.Helper()
 	if sr.AccError() == nil {
 		t.Errorf("should have had an accumulated error")
 	}
diff --git a/mp4/dref_test.go b/mp4/dref_test.go
index af09c39..481d78d 100644
--- a/mp4/dref_test.go
+++ b/mp4/dref_test.go
@@ -1,10 +1,30 @@
 package mp4
 
 import (
+	"encoding/hex"
 	"testing"
+
+	"github.com/Eyevinn/mp4ff/bits"
 )
 
+const data = `000000326472656600000000000000010000002275726c200000000168747470733a2f2f666c7573736f6e69632e636f6d2f`
+
 func TestDref(t *testing.T) {
 	dref := CreateDref()
 	boxDiffAfterEncodeAndDecode(t, dref)
 }
+
+func TestDrefDecode(t *testing.T) {
+	d, err := hex.DecodeString(data)
+	if err != nil {
+		t.Error(err)
+	}
+	sr := bits.NewFixedSliceReader(d)
+	box, err := DecodeBoxSR(0, sr)
+	if err != nil {
+		t.Error(err)
+	}
+	if box.Type() != "dref" {
+		t.Errorf("Expected 'dref', got %s", box.Type())
+	}
+}
diff --git a/mp4/testdata/golden_init_cenc_cmfv_dump.txt b/mp4/testdata/golden_init_cenc_cmfv_dump.txt
index 6bd65f8..e29b1bb 100644
--- a/mp4/testdata/golden_init_cenc_cmfv_dump.txt
+++ b/mp4/testdata/golden_init_cenc_cmfv_dump.txt
@@ -29,8 +29,7 @@
         [vmhd] size=20 version=0 flags=000000
         [dinf] size=36
           [dref] size=28 version=0 flags=000000
-            [url ] size=12
-             - location: ""
+            [url ] size=12 version=0 flags=000001
         [stbl] size=319
           [stsd] size=243 version=0 flags=000000
             [encv] size=227
diff --git a/mp4/testdata/golden_init_prog_mp4_dump.txt b/mp4/testdata/golden_init_prog_mp4_dump.txt
index 9af5e3a..cbd23ee 100644
--- a/mp4/testdata/golden_init_prog_mp4_dump.txt
+++ b/mp4/testdata/golden_init_prog_mp4_dump.txt
@@ -35,8 +35,7 @@
         [vmhd] size=20 version=0 flags=000001
         [dinf] size=36
           [dref] size=28 version=0 flags=000000
-            [url ] size=12
-             - location: ""
+            [url ] size=12 version=0 flags=000001
         [stbl] size=4655
           [stsd] size=183 version=0 flags=000000
             [avc1] size=167
diff --git a/mp4/testdata/golden_init_video_mp4_dump.txt b/mp4/testdata/golden_init_video_mp4_dump.txt
index 32210c3..767d328 100644
--- a/mp4/testdata/golden_init_video_mp4_dump.txt
+++ b/mp4/testdata/golden_init_video_mp4_dump.txt
@@ -36,8 +36,7 @@
         [vmhd] size=20 version=0 flags=000001
         [dinf] size=36
           [dref] size=28 version=0 flags=000000
-            [url ] size=12
-             - location: ""
+            [url ] size=12 version=0 flags=000001
         [stbl] size=237
           [stsd] size=161 version=0 flags=000000
             [avc3] size=145
diff --git a/mp4/url_test.go b/mp4/url_test.go
index 4a64b3a..73012c8 100644
--- a/mp4/url_test.go
+++ b/mp4/url_test.go
@@ -1,7 +1,11 @@
 package mp4
 
 import (
+	"bytes"
+	"encoding/hex"
 	"testing"
+
+	"github.com/Eyevinn/mp4ff/bits"
 )
 
 func TestUrl(t *testing.T) {
@@ -14,3 +18,64 @@ func TestUrl(t *testing.T) {
 
 	boxDiffAfterEncodeAndDecode(t, urlBox)
 }
+
+func TestUrlDecode(t *testing.T) {
+	cases := []struct {
+		desc              string
+		data              string
+		wantedFlags       uint32
+		wantedLocation    string
+		NoLocation        bool
+		NoZeroTermination bool
+	}{
+		{
+			desc:              "self-contained, with location and zero termination",
+			data:              `0000002375726c200000000168747470733a2f2f666c7573736f6e69632e636f6d2f00`,
+			wantedFlags:       0x00001,
+			wantedLocation:    "",
+			NoLocation:        false,
+			NoZeroTermination: false,
+		},
+		{
+			desc:              "self-contained,  with location but no zero termination",
+			data:              `0000002275726c200000000168747470733a2f2f666c7573736f6e69632e636f6d2f`,
+			wantedFlags:       0x00001,
+			wantedLocation:    "",
+			NoLocation:        false,
+			NoZeroTermination: true,
+		},
+		{
+			desc:              "self-contained,  without location",
+			data:              `0000000c75726c2000000001`,
+			wantedFlags:       0x00001,
+			wantedLocation:    "",
+			NoLocation:        false,
+			NoZeroTermination: true,
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			d, err := hex.DecodeString(c.data)
+			if err != nil {
+				t.Error(err)
+			}
+			sr := bits.NewFixedSliceReader(d)
+			box, err := DecodeBoxSR(0, sr)
+			if err != nil {
+				t.Error(err)
+			}
+			if box.Type() != "url " {
+				t.Errorf("Expected 'url ', got %s", box.Type())
+			}
+			urlBox := box.(*URLBox)
+			o := bytes.Buffer{}
+			err = urlBox.Encode(&o)
+			if err != nil {
+				t.Error(err)
+			}
+			if !bytes.Equal(d, o.Bytes()) {
+				t.Errorf("Encode mismatch: got %s, wanted %s", hex.EncodeToString(o.Bytes()), c.data)
+			}
+		})
+	}
+}