Repository: Eyevinn/mp4ff (go)

Issue:
potential bug in stsc
I was looking at refactoring the following code:

https://github.com/Eyevinn/mp4ff/blob/36777400d47b11751c670be70d9d0c78a41115ff/mp4/stsc.go#L70-L77

and noticed that because it referenced `b.SampleDescriptionID[i]` inside the loop where it was iterating over `j`, it was assigning the same thing to the same element over and over again. I assume that isn't the correct logic, but didn't know what it should be.

Pull request description:
fix: stsc correct handling of multiple sample description IDs
stsc did not write the first sample description ID value to correctly as a new value appeared.

Fixes #398.

Patch:
diff --git a/mp4/stsc.go b/mp4/stsc.go
index ffaac28..9671d9e 100644
--- a/mp4/stsc.go
+++ b/mp4/stsc.go
@@ -49,8 +49,12 @@ func DecodeStscSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, err
 	b := StscBox{
 		Version: byte(versionAndFlags >> 24),
 		Flags:   versionAndFlags & flagsMask,
-		Entries: make([]StscEntry, entryCount),
 	}
+	if hdr.Size != b.expectedSize(int(entryCount)) {
+		return nil, fmt.Errorf("invalid stsc box size")
+	}
+
+	b.Entries = make([]StscEntry, entryCount)
 
 	var accSampleNr uint32 = 1
 
@@ -70,7 +74,7 @@ func DecodeStscSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, err
 				if b.singleSampleDescriptionID != 0 {
 					b.SampleDescriptionID = make([]uint32, entryCount)
 					for j := 0; j < i; j++ {
-						b.SampleDescriptionID[i] = sdi
+						b.SampleDescriptionID[j] = b.singleSampleDescriptionID
 					}
 					b.singleSampleDescriptionID = 0
 				}
@@ -88,7 +92,11 @@ func (b *StscBox) Type() string {
 
 // Size - box-specific size
 func (b *StscBox) Size() uint64 {
-	return uint64(boxHeaderSize + 8 + len(b.Entries)*12)
+	return b.expectedSize(len(b.Entries))
+}
+
+func (b *StscBox) expectedSize(nrEntries int) uint64 {
+	return uint64(boxHeaderSize + 8 + nrEntries*12)
 }
 
 // Encode - write box to w


Test patch:
diff --git a/mp4/stsc_test.go b/mp4/stsc_test.go
index b5e34aa..e5ed5fe 100644
--- a/mp4/stsc_test.go
+++ b/mp4/stsc_test.go
@@ -1,6 +1,7 @@
 package mp4
 
 import (
+	"bytes"
 	"testing"
 
 	"github.com/go-test/deep"
@@ -171,3 +172,21 @@ func TestGetChunk(t *testing.T) {
 		}
 	}
 }
+
+func TestStscSampleDescriptionID(t *testing.T) {
+	box := StscBox{}
+	_ = box.AddEntry(1, 256, 1)
+	_ = box.AddEntry(2, 192, 1)
+	_ = box.AddEntry(3, 128, 2)
+	boxDiffAfterEncodeAndDecode(t, &box)
+}
+
+func TestBadSizeStsc(t *testing.T) {
+	// raw stsc box with size 16, but with one entry, so its size should be 28ß
+	raw := []byte{0x00, 0x00, 0x00, 0x10, 's', 't', 's', 'c', 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}
+	buf := bytes.NewBuffer(raw)
+	_, err := DecodeBox(0, buf)
+	if err == nil {
+		t.Error("expected invalid size error")
+	}
+}