Merge pull request #374 from Eyevinn/fix-multi-mdat

fix: allow additional empty mdat boxes in progressive files

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Allow missing optional DecoderSpecificInfo
+- Avoid mp4.File.Mdat pointing to an empty mdat box
 
 - Nothing yet
 

mp4/boxsr.go

@@ -226,6 +226,16 @@ LoopBoxes:
 				if lastBoxType != "moof" {
 					return nil, fmt.Errorf("does not support %v between moof and mdat", lastBoxType)
 				}
+			} else {
+				if f.Mdat != nil {
+					oldPayloadSize := f.Mdat.Size() - f.Mdat.HeaderSize()
+					newMdat := box.(*MdatBox)
+					newPayloadSize := newMdat.Size() - newMdat.HeaderSize()
+					if oldPayloadSize > 0 && newPayloadSize > 0 {
+						return nil, fmt.Errorf("only one non-empty mdat box supported (payload sizes %d and %d)",
+							oldPayloadSize, newPayloadSize)
+					}
+				}
 			}
 		case "moof":
 			moof := box.(*MoofBox)

mp4/file.go

@@ -27,7 +27,7 @@ import (
 type File struct {
 	Ftyp         *FtypBox
 	Moov         *MoovBox
-	Mdat         *MdatBox        // Only used for non-fragmented files
+	Mdat         *MdatBox        // mdat box for non-fragmented files. Extra empty boxes allowed.
 	Init         *InitSegment    // Init data (ftyp + moov for fragmented file)
 	Sidx         *SidxBox        // The first sidx box for a DASH OnDemand file
 	Sidxs        []*SidxBox      // All sidx boxes for a DASH OnDemand file
@@ -196,6 +196,16 @@ LoopBoxes:
 				if lastBoxType != "moof" {
 					return nil, fmt.Errorf("does not support %v between moof and mdat", lastBoxType)
 				}
+			} else {
+				if f.Mdat != nil {
+					oldPayloadSize := f.Mdat.Size() - f.Mdat.HeaderSize()
+					newMdat := box.(*MdatBox)
+					newPayloadSize := newMdat.Size() - newMdat.HeaderSize()
+					if oldPayloadSize > 0 && newPayloadSize > 0 {
+						return nil, fmt.Errorf("only one non-empty mdat box supported (payload sizes %d and %d)",
+							oldPayloadSize, newPayloadSize)
+					}
+				}
 			}
 		case "moof":
 			moof := box.(*MoofBox)
@@ -294,8 +304,10 @@ func (f *File) AddChild(child Box, boxStartPos uint64) {
 		frag := currSeg.LastFragment()
 		frag.AddChild(moof)
 	case *MdatBox:
-		if !f.isFragmented {
-			f.Mdat = box
+		if !f.isFragmented { // Only add if previous mdat is nil or empty
+			if f.Mdat == nil || f.Mdat.Size()-f.Mdat.HeaderSize() == 0 {
+				f.Mdat = box
+			}
 		} else {
 			currentFragment := f.LastSegment().LastFragment()
 			currentFragment.AddChild(box)

mp4/file_test.go

@@ -2,6 +2,7 @@ package mp4
 
 import (
 	"bytes"
+	"fmt"
 	"os"
 	"testing"
 
@@ -324,3 +325,57 @@ func TestUpdateSidx(t *testing.T) {
 		t.Error("sidx should be present")
 	}
 }
+
+func TestEmptyMdat(t *testing.T) {
+	testCases := []struct {
+		desc          string
+		mdatSizes     []uint64
+		expectedError string
+	}{
+		{desc: "2 non-empty", mdatSizes: []uint64{24, 16},
+			expectedError: "only one non-empty mdat box supported (payload sizes 16 and 8)"},
+		{desc: "empty + normal", mdatSizes: []uint64{8, 16}, expectedError: ""},
+		{desc: "normal+empty", mdatSizes: []uint64{16, 8}, expectedError: ""},
+		{desc: "empty+normal+empty", mdatSizes: []uint64{8, 16, 8}, expectedError: ""},
+	}
+	for _, tc := range testCases {
+		for _, readSlice := range []bool{true, false} {
+			t.Run(fmt.Sprintf("%s_readSlice_%t", tc.desc, readSlice), func(t *testing.T) {
+				buf := bytes.Buffer{}
+				for _, mdatSize := range tc.mdatSizes {
+					mdat := &MdatBox{}
+					if mdatSize > 8 {
+						mdat.Data = make([]byte, mdatSize-8)
+					}
+					err := mdat.Encode(&buf)
+					if err != nil {
+						t.Error(err)
+					}
+				}
+				var decFile *File
+				var err error
+				if readSlice {
+					sr := bits.NewFixedSliceReader(buf.Bytes())
+					decFile, err = DecodeFileSR(sr)
+				} else {
+					decFile, err = DecodeFile(&buf)
+				}
+				if tc.expectedError != "" {
+					if err == nil {
+						t.Error("expected error")
+					} else if err.Error() != tc.expectedError {
+						t.Errorf("expected error %s, got %s", tc.expectedError, err.Error())
+					}
+					return
+				}
+				if err != nil {
+					t.Error(err)
+				}
+				mdat := decFile.Mdat
+				if mdat.Size() == 8 {
+					t.Error("f.Mdat points to empty file although there is a non-empty mdat")
+				}
+			})
+		}
+	}
+}