commit c5209d277a60973090abb0eaa9363428b111400f
parent 889b534a511e17176284dc1662d4a4128445c1bd
Author: Oliver Lowe <o@olowe.co>
Date: Sun, 19 Jan 2025 21:54:29 +1100
mpegts: test for expected PCR base, extension values
Parsing the base and extension ticks of the PCR was actually
incorrect. Whilst we were consistently storing and decoding values,
they were incorrect values. This patch adds tests which concretely
exposed the bug spotted by @kevmo314. Thanks again!
Diffstat:
2 files changed, 94 insertions(+), 31 deletions(-)
diff --git a/mpegts/codec.go b/mpegts/codec.go
@@ -115,16 +115,26 @@ func parseAdaptationField(buf []byte) *Adaptation {
return &af
}
-// 0 pppp pppp
-// 1 pppp pppp
-// 2 pppp pppp
-// 3 pppp pppp
-// 4 prrr rrre
-// 5 eeee eeee
+// parsePCR parses the encoded PCR from a.
+// The 33-bit base and the 9-bit extension
+// are stored in a 6 byte array with the following bit layout,
+// where "b" stands for "base", "r" for reserved bits, and "e" for extension.
+//
+// 0 bbbb bbbb
+// 1 bbbb bbbb
+// 2 bbbb bbbb
+// 3 bbbb bbbb
+// 4 brrr rrre
+// 5 eeee eeee
func parsePCR(a [6]byte) PCR {
- b := []byte{0, 0, 0, a[0], a[1], a[2], a[3], a[4] & 0x80}
- base := binary.BigEndian.Uint64(b) >> 7
- // next 6 bits reserved, so remaining 1 bit in a[5] and all of a[6] have the extension.
+ // we only want the left-most bit.
+ // 6 bits are reserved and the right-most bit is part of extension.
+ b := [8]byte{0, 0, 0, a[0], a[1], a[2], a[3], a[4] & 0x80}
+ base := binary.BigEndian.Uint64(b[:])
+ base = base >> 7 // trim masked reserved, extension bits
+ // next 6 bits of a[4] are reserved, so right-most bit in a[4]
+ // and all of a[5] have the extension.
+
ext := binary.BigEndian.Uint16([]byte{a[4] & 0x01, a[5]})
return PCR{base, ext}
}
@@ -266,25 +276,52 @@ func Encode(w io.Writer, p *Packet) error {
return err
}
+const (
+ baseMax = 8589934592 - 1 // max 33-bit uint
+ extensionMax = 512 - 1 // max 9-bit uint
+)
+
func putPCR(b []byte, pcr *PCR) error {
if len(b) != 6 {
return fmt.Errorf("need %d bytes, got %d", 6, len(b))
}
- max := uint64(8589934592 - 1) // max 33-bit int
- if pcr.Base > max {
- return fmt.Errorf("base %d larger than max %d", pcr.Base, max)
+ if pcr.Base > baseMax {
+ return fmt.Errorf("base %d larger than max %d", pcr.Base, baseMax)
+ } else if pcr.Extension > extensionMax {
+ return fmt.Errorf("extension %d larger than max %d", pcr.Extension, extensionMax)
}
- ubuf := make([]byte, 8)
+
+ ubuf := make([]byte, 8, 8)
binary.BigEndian.PutUint64(ubuf, pcr.Base)
- ubuf = ubuf[3:] // we only want 33 bits, so get 4 + 1 bytes (32+1 bits)
- copy(b, ubuf)
- b[4] |= 0x7e // toggle 6 reserved bits
+ // we're only working with 33 bits, so slice off the first 3
+ // bytes to get 4 + 1 bytes (32+1 bits)
+ ubuf = ubuf[3:]
- var emax uint16 = 512 - 1 // max 9-bit int
- if pcr.Extension > emax {
- return fmt.Errorf("extension %d larger than max %d", pcr.Extension, emax)
- }
- b[4] |= byte(pcr.Extension >> 8)
- b[5] = byte(pcr.Extension)
+ // now pack 33 bits from ubuf into b[:4].
+ // The 33rd bit of our 33-bit integer is in the first byte: 0b00000001.
+ // We're packing bits from left to right, so shift it left and assign to b[0].
+ b[0] = ubuf[0] << 7
+
+ // We have 7 bits free in b[0], so get 7 bits from ubuf[1] and pack it into b[0].
+ b[0] |= ubuf[1] >> 1
+
+ // 1 bit left in ubuf[1]; put it in the next dest byte.
+ // Rinse and repeat until we're out of bits.
+ b[1] = ubuf[1] << 7
+ b[1] |= ubuf[2] >> 1
+ b[2] = ubuf[2] << 7
+ b[2] |= ubuf[3] >> 1
+ b[3] = ubuf[3] << 7
+ b[3] |= ubuf[4] >> 1
+ b[4] = ubuf[4] << 7
+ // No more base bits to pack.
+
+ // Next, toggle the 6 reserved bits.
+ b[4] |= 0b01111110
+
+ var ext [2]byte
+ binary.BigEndian.PutUint16(ext[:], pcr.Extension)
+ b[4] |= ext[0]
+ b[5] = ext[1]
return nil
}
diff --git a/mpegts/codec_test.go b/mpegts/codec_test.go
@@ -119,15 +119,41 @@ func TestScanner(t *testing.T) {
}
func TestPCR(t *testing.T) {
- a := [6]byte{0x00, 0x24, 0x52, 0xd4, 0x7e, 0x00}
- pcr := parsePCR(a)
-
- var got [6]byte
- if err := putPCR(got[:], &pcr); err != nil {
- t.Errorf("put pcr: %v", err)
+ var tests = []struct {
+ name string
+ encoded [6]byte
+ want PCR
+ }{
+ {
+ "zero",
+ [6]byte{0, 0, 0, 0, 0b01111110, 0}, // 6 reserved bits toggled
+ PCR{},
+ },
+ {
+ "max base", // 2^33 - 1
+ [6]byte{0xff, 0xff, 0xff, 0xff, 0xfe, 0x00},
+ PCR{8589934591, 0},
+ },
+ {
+ "max extension", // 2^9 - 1
+ [6]byte{0, 0, 0, 0, 0x7f, 0xff},
+ PCR{0, 511},
+ },
}
- if got != a {
- t.Errorf("PCR differs after decode, re-encode")
- t.Errorf("putPCR(buf, %v) = %08b, want %08b", pcr, got, a)
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ pcr := parsePCR(tt.encoded)
+ if pcr != tt.want {
+ t.Errorf("parsePCR(%v) = %v, want %v", tt.encoded, pcr, tt.want)
+ }
+
+ var a [6]byte
+ if err := putPCR(a[:], &pcr); err != nil {
+ t.Fatalf("put PCR: %v", err)
+ }
+ if a != tt.encoded {
+ t.Errorf("re-encoded pcr is %v, want %v", a, tt.encoded)
+ }
+ })
}
}
