codec/h264/h264dec: added tests for pps parsing found in pps.go and made necessary changes

Changes to get successful parsing included modification to moreRBSPData, and as a result
the Off function from the bits.BitReader. A couple of basic PPS tests have been added, but
more should be added once we know the scaling list parsing works.

codec/h264/h264dec/bits/bitreader.go

@@ -164,6 +164,11 @@ func (br *BitReader) ByteAligned() bool {
 	return br.bits == 0
 }
 
+// Off returns the current offset from the starting bit of the current byte.
+func (br *BitReader) Off() int {
+	return br.bits
+}
+
 // BytesRead returns the number of bytes that have been read by the BitReader.
 func (br *BitReader) BytesRead() int {
 	return br.nRead

codec/h264/h264dec/parse_test.go

@@ -89,13 +89,13 @@ func TestReadSe(t *testing.T) {
 		in   []byte // Bitstring to read.
 		want int    // Expected value from se(v) parsing process.
 	}{
-		{[]byte{0x80}, 0},
+		{[]byte{0x80}, 0},  // Bit string: 1, codeNum: 0, syntax element val: 0
-		{[]byte{0x40}, 1},
+		{[]byte{0x40}, 1},  // Bit string: 010, codeNum: 1, syntax element val: 1
-		{[]byte{0x60}, -1},
+		{[]byte{0x60}, -1}, // Bit string: 011, codeNum: 2, syntax element val: -1
-		{[]byte{0x20}, 2},
+		{[]byte{0x20}, 2},  // Bit string: 00100, codeNum: 3, syntax element val: 2
-		{[]byte{0x28}, -2},
+		{[]byte{0x28}, -2}, // Bit string: 00101, codeNum: 4, syntax element val: -2
-		{[]byte{0x30}, 3},
+		{[]byte{0x30}, 3},  // Bit string: 00110, codeNum: 5, syntax element val: 3
-		{[]byte{0x38}, -3},
+		{[]byte{0x38}, -3}, // Bit string: 00111, codeNum: 6, syntax element val: -3
 	}
 
 	for i, test := range tests {

codec/h264/h264dec/pps.go

@@ -14,9 +14,8 @@ import (
 type PPS struct {
 	ID, SPSID                         int
 	EntropyCodingMode                 int
-	NumSliceGroupsMinus1              int
 	BottomFieldPicOrderInFramePresent bool
-	NumSlicGroupsMinus1               int
+	NumSliceGroupsMinus1              int
 	SliceGroupMapType                 int
 	RunLengthMinus1                   []int
 	TopLeft                           []int
@@ -41,14 +40,10 @@ type PPS struct {
 	SecondChromaQpIndexOffset         int
 }
 
-func NewPPS(sps *SPS, rbsp []byte, showPacket bool) (*PPS, error) {
+func NewPPS(br *bits.BitReader, chromaFormat int) (*PPS, error) {
-	logger.Printf("debug: PPS RBSP %d bytes %d bits == \n", len(rbsp), len(rbsp)*8)
-	logger.Printf("debug: \t%#v\n", rbsp[0:8])
 	pps := PPS{}
-	// TODO: give this io.Reader
-	br := bits.NewBitReader(nil)
-
 	var err error
+
 	pps.ID, err = readUe(br)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not parse ID")
@@ -84,10 +79,11 @@ func NewPPS(sps *SPS, rbsp []byte, showPacket bool) (*PPS, error) {
 
 		if pps.SliceGroupMapType == 0 {
 			for iGroup := 0; iGroup <= pps.NumSliceGroupsMinus1; iGroup++ {
-				pps.RunLengthMinus1[iGroup], err = readUe(br)
+				b, err := readUe(br)
 				if err != nil {
 					return nil, errors.Wrap(err, "could not parse RunLengthMinus1")
 				}
+				pps.RunLengthMinus1 = append(pps.RunLengthMinus1, b)
 			}
 		} else if pps.SliceGroupMapType == 2 {
 			for iGroup := 0; iGroup < pps.NumSliceGroupsMinus1; iGroup++ {
@@ -195,7 +191,7 @@ func NewPPS(sps *SPS, rbsp []byte, showPacket bool) (*PPS, error) {
 
 		if pps.PicScalingMatrixPresent {
 			v := 6
-			if sps.ChromaFormat != chroma444 {
+			if chromaFormat != chroma444 {
 				v = 2
 			}
 			for i := 0; i < 6+(v*pps.Transform8x8Mode); i++ {
@@ -222,18 +218,12 @@ func NewPPS(sps *SPS, rbsp []byte, showPacket bool) (*PPS, error) {
 					}
 				}
 			}
-			pps.SecondChromaQpIndexOffset, err = readSe(br)
-			if err != nil {
-				return nil, errors.New("could not parse SecondChromaQpIndexOffset")
-			}
 		}
-		moreRBSPData(br)
+		pps.SecondChromaQpIndexOffset, err = readSe(br)
-		// rbspTrailingBits()
+		if err != nil {
-	}
+			return nil, errors.New("could not parse SecondChromaQpIndexOffset")
-
+		}
-	if showPacket {
-		debugPacket("PPS", pps)
 	}
+	moreRBSPData(br)
 	return &pps, nil
-
 }

codec/h264/h264dec/pps_test.go

@@ -0,0 +1,147 @@
+package h264dec
+
+import (
+	"bytes"
+	"errors"
+	"reflect"
+	"testing"
+
+	"bitbucket.org/ausocean/av/codec/h264/h264dec/bits"
+)
+
+func TestNewPPS(t *testing.T) {
+	// TODO: add test with scaling list once we have a test for scalingList func.
+	tests := []struct {
+		in           string
+		chromaFormat int
+		want         PPS
+	}{
+		{
+			in: "1" + // ue(v) pic_parameter_set_id = 0
+				"1" + // ue(v) seq_parameter_set_id = 0
+				"1" + // u(1) entropy_coding_mode_flag = 1
+				"0" + // u(1) pic_order_present_flag = 0
+				"1" + // ue(v) num_slice_groups_minus1 = 0
+				"1" + // ue(v) num_ref_idx_L0_active_minus1 = 0
+				"1" + // ue(v) num_ref_idx_L1_active_minus1 = 0
+				"1" + // u(1) weighted_pred_flag = 1
+				"00" + // u(2) weighted_bipred_idc = 0
+				"1" + // se(v) pic_init_qp_minus26 =  0
+				"1" + // se(v) pic_init_qs_minus26 =  0
+				"1" + // se(v) chroma_qp_index_offset =  0
+				"1" + // u(1) deblocking_filter_control_present_flag =  1
+				"0" + // u(1) constrained_intra_pred_flag =  0
+				"0" + // u(1) redundant_pic_cnt_present_flag =  0
+				"10000000", // rbspTrailingBits
+			want: PPS{
+				ID:                                0,
+				SPSID:                             0,
+				EntropyCodingMode:                 1,
+				BottomFieldPicOrderInFramePresent: false,
+				NumSliceGroupsMinus1:              0,
+				NumRefIdxL0DefaultActiveMinus1:    0,
+				NumRefIdxL1DefaultActiveMinus1:    0,
+				WeightedPred:                      true,
+				WeightedBipred:                    0,
+				PicInitQpMinus26:                  0,
+				PicInitQsMinus26:                  0,
+				ChromaQpIndexOffset:               0,
+				DeblockingFilterControlPresent:    true,
+				ConstrainedIntraPred:              false,
+				RedundantPicCntPresent:            false,
+			},
+		},
+		{
+			in: "1" + // ue(v) pic_parameter_set_id = 0
+				"1" + // ue(v) seq_parameter_set_id = 0
+				"1" + // u(1) entropy_coding_mode_flag = 1
+				"1" + // u(1) bottom_field_pic_order_in_frame_present_flag = 1
+				"010" + // ue(v) num_slice_groups_minus1 = 1
+				"1" + // ue(v) slice_group_map_type = 0
+				"1" + // ue(v) run_length_minus1[0] = 0
+				"1" + // ue(v) run_length_minus1[1] = 0
+				"1" + // ue(v) num_ref_idx_L0_active_minus1 = 0
+				"1" + // ue(v) num_ref_idx_L1_active_minus1 = 0
+				"1" + // u(1) weighted_pred_flag = 0
+				"00" + // u(2) weighted_bipred_idc = 0
+				"011" + // se(v) pic_init_qp_minus26 = -1
+				"010" + // se(v) pic_init_qs_minus26 = 1
+				"00100" + // se(v) chroma_qp_index_offset = 2
+				"0" + // u(1) deblocking_filter_control_present_flag =0
+				"0" + // u(1) constrained_intra_pred_flag=0
+				"0" + // u(1) redundant_pic_cnt_present_flag=0
+				"0" + // u(1) transform_8x8_mode_flag=0
+				"0" + // u(1) pic_scaling_matrix_present_flag=0
+				"00100" + // se(v) second_chroma_qp_index_offset=2
+				"10000", // stop bit and trailing bits
+			want: PPS{
+				ID:                                0,
+				SPSID:                             0,
+				EntropyCodingMode:                 1,
+				BottomFieldPicOrderInFramePresent: true,
+				NumSliceGroupsMinus1:              1,
+				RunLengthMinus1:                   []int{0, 0},
+				NumRefIdxL0DefaultActiveMinus1:    0,
+				NumRefIdxL1DefaultActiveMinus1:    0,
+				WeightedPred:                      true,
+				WeightedBipred:                    0,
+				PicInitQpMinus26:                  -1,
+				PicInitQsMinus26:                  1,
+				ChromaQpIndexOffset:               2,
+				DeblockingFilterControlPresent:    false,
+				ConstrainedIntraPred:              false,
+				RedundantPicCntPresent:            false,
+				Transform8x8Mode:                  0,
+				PicScalingMatrixPresent:           false,
+				SecondChromaQpIndexOffset:         2,
+			},
+		},
+	}
+
+	for i, test := range tests {
+		bin, err := binToSlice(test.in)
+		if err != nil {
+			t.Fatalf("error: %v converting binary string to slice for test: %d", err, i)
+		}
+
+		pps, err := NewPPS(bits.NewBitReader(bytes.NewReader(bin)), test.chromaFormat)
+		if err != nil {
+			t.Fatalf("did not expect error: %v for test: %d", err, i)
+		}
+
+		if !reflect.DeepEqual(test.want, *pps) {
+			t.Errorf("did not get expected result for test: %d.\nGot: %+v\nWant: %+v\n", i, *pps, test.want)
+		}
+	}
+}
+
+// binToSlice is a helper function to convert a string of binary into a
+// corresponding byte slice, e.g. "0100 0001 1000 1100" => {0x41,0x8c}.
+// Spaces in the string are ignored.
+func binToSlice(s string) ([]byte, error) {
+	var (
+		a     byte = 0x80
+		cur   byte
+		bytes []byte
+	)
+
+	for _, c := range s {
+		switch c {
+		case ' ':
+			continue
+		case '1':
+			cur |= a
+		case '0':
+		default:
+			return nil, errors.New("invalid binary string")
+		}
+
+		a >>= 1
+		if a == 0 {
+			bytes = append(bytes, cur)
+			cur = 0
+			a = 0x80
+		}
+	}
+	return bytes, nil
+}

codec/h264/h264dec/read.go

@@ -71,7 +71,7 @@ func (h *H264Reader) Start() {
 		case naluTypePPS:
 			videoStream := h.VideoStreams[len(h.VideoStreams)-1]
 			// TODO: handle this error
-			videoStream.PPS, _ = NewPPS(videoStream.SPS, nalUnit.RBSP(), false)
+			videoStream.PPS, _ = NewPPS(nil, videoStream.SPS.ChromaFormat)
 		case naluTypeSliceIDRPicture:
 			fallthrough
 		case naluTypeSliceNonIDRPicture:
@@ -177,23 +177,63 @@ func isEmpty3Byte(buf []byte) bool {
 	return true
 }
 
-// TODO: complete this.
 func moreRBSPData(br *bits.BitReader) bool {
-	// Read until the least significant bit of any remaining bytes
+	// If we get an error then we must at end of NAL unit or end of stream, so
-	// If the least significant bit is 1, that marks the first bit
+	// return false.
-	// of the rbspTrailingBits() struct. If the bits read is more
+	b, err := br.PeekBits(1)
-	// than 0, then there is more RBSP data
+	if err != nil {
-	var bits uint64
+		return false
-	cnt := 0
-	for bits != 1 {
-		if _, err := br.ReadBits(8); err != nil {
-			logger.Printf("moreRBSPData error: %v\n", err)
-			return false
-		}
-		cnt++
 	}
-	logger.Printf("moreRBSPData: read %d additional bits\n", cnt)
+
-	return cnt > 0
+	// If b is not 1, then we don't have a stop bit and therefore there is more
+	// data so return true.
+	if b == 0 {
+		return true
+	}
+
+	// If we have a stop bit and trailing zeros then we're okay, otherwise return
+	// now, we haven't found the end.
+	b, err = br.PeekBits(8 - br.Off())
+	if err != nil {
+		return false
+	}
+	rem := 0x01 << uint(7-br.Off())
+	if int(b) != rem {
+		return true
+	}
+
+	// If we try to read another bit but get EOF then we must be at the end of the
+	// NAL or stream.
+	_, err = br.PeekBits(9 - br.Off())
+	if err != nil {
+		return false
+	}
+
+	// Do we have some trailing 0 bits, and then a 24-bit start code ? If so, it
+	// there must not be any more RBSP data left.
+	// If we get an error from the Peek, then there must not be another NAL, and
+	// there must be some more RBSP, because trailing bits do not extend past the
+	// byte in which the stop bit is found.
+	b, err = br.PeekBits(8 - br.Off() + 24)
+	if err != nil {
+		return true
+	}
+	rem = (0x01 << uint((7-br.Off())+24)) | 0x01
+	if int(b) == rem {
+		return false
+	}
+
+	// Similar check to above, but this time checking for 32-bit start code.
+	b, err = br.PeekBits(8 - br.Off() + 32)
+	if err != nil {
+		return true
+	}
+	rem = (0x01 << uint((7-br.Off())+32)) | 0x01
+	if int(b) == rem {
+		return false
+	}
+
+	return true
 }
 
 type field struct {

codec/h264/h264dec/read_test.go

@@ -0,0 +1,52 @@
+package h264dec
+
+import (
+	"bytes"
+	"testing"
+
+	"bitbucket.org/ausocean/av/codec/h264/h264dec/bits"
+)
+
+func TestMoreRBSPData(t *testing.T) {
+	tests := []struct {
+		in   string
+		want bool
+	}{
+		{
+			in:   "00000100",
+			want: true,
+		},
+		{
+			in:   "10000100",
+			want: true,
+		},
+		{
+			in:   "10000000",
+			want: false,
+		},
+		{
+			in:   "10000000 00000000 00000000 00000001",
+			want: false,
+		},
+		{
+			in:   "10000000 00000000 00000000 00000000 00000001",
+			want: false,
+		},
+		{
+			in:   "10000000 00000000",
+			want: true,
+		},
+	}
+
+	for i, test := range tests {
+		b, err := binToSlice(test.in)
+		if err != nil {
+			t.Fatalf("unexpected binToSlice error: %v for test: %d", err, i)
+		}
+
+		got := moreRBSPData(bits.NewBitReader(bytes.NewReader(b)))
+		if got != test.want {
+			t.Errorf("unexpected result for test: %d\nGot: %v\nWant: %v\n", i, got, test.want)
+		}
+	}
+}