protocol/rtcp: addressing PR feedback

protocol/rtcp/client.go

@@ -4,7 +4,7 @@ NAME
 
 DESCRIPTION
   Client.go provides an implemntation of a basic RTCP Client that will send
-	receiver reports, and receive sender reports to parse relevant statistics.
+  receiver reports, and receive sender reports to parse relevant statistics.
 
 AUTHORS
   Saxon A. Nelson-Milton <saxon@ausocean.org>
@@ -20,7 +20,7 @@ LICENSE
   It is distributed in the hope that it will be useful, but WITHOUT
   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
   FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
- 	for more details.
+  for more details.
 
   You should have received a copy of the GNU General Public License
   in gpl.txt.  If not, see http://www.gnu.org/licenses.
@@ -226,18 +226,11 @@ func (c *Client) formPayload(r *ReceiverReport, d *SourceDescription) []byte {
 // parse will read important statistics from sender reports.
 func (c *Client) parse(buf []byte) {
 	c.markReceivedTime()
-	msw, lsw, err := Timestamp(buf)
+	t, err := Timestamp(buf)
 	if err != nil {
 		c.ErrChan <- errors.New(fmt.Sprintf("could not get timestamp from sender report, failed with error: %v", err))
 	}
-	c.setSenderTs(
+	c.setSenderTs(t)
-		struct {
-			msw uint32
-			lsw uint32
-		}{
-			msw,
-			lsw,
-		})
 }
 
 // SetSequence will allow updating of the highest sequence number received
@@ -263,10 +256,10 @@ func (c *Client) jitter() uint32 {
 }
 
 // setSenderTs allows us to safely set the current sender report timestamp.
-func (c *Client) setSenderTs(t struct{ msw, lsw uint32 }) {
+func (c *Client) setSenderTs(t NTPTimestamp) {
 	c.mu.Lock()
-	binary.BigEndian.PutUint32(c.senderTs[:], t.msw)
+	binary.BigEndian.PutUint32(c.senderTs[:], t.MSW)
-	binary.BigEndian.PutUint32(c.senderTs[4:], t.lsw)
+	binary.BigEndian.PutUint32(c.senderTs[4:], t.LSW)
 	c.mu.Unlock()
 }
 

protocol/rtcp/parse.go

@@ -19,7 +19,7 @@ LICENSE
   It is distributed in the hope that it will be useful, but WITHOUT
   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
   FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
- 	for more details.
+  for more details.
 
   You should have received a copy of the GNU General Public License
   in gpl.txt.  If not, see http://www.gnu.org/licenses.
@@ -32,23 +32,29 @@ import (
 	"errors"
 )
 
+// NTPTimestamp describes the NTP timestamp format (http://www.beaglesoft.com/Manual/page53.htm)
+type NTPTimestamp struct {
+	MSW uint32
+	LSW uint32
+}
+
 // Timestamp gets the timestamp from a receiver report and returns it as the most
 // significant word, and the least significant word. If the given bytes do not
 // represent a valid receiver report, an error is returned.
-func Timestamp(buf []byte) (msw, lsw uint32, err error) {
+func Timestamp(buf []byte) (NTPTimestamp, error) {
 	if len(buf) < 4 {
-		return 0, 0, errors.New("bad RTCP packet, not of sufficient length")
+		return NTPTimestamp{}, errors.New("bad RTCP packet, not of sufficient length")
 	}
 	if (buf[0]&0xc0)>>6 != rtcpVer {
-		return 0, 0, errors.New("incompatible RTCP version")
+		return NTPTimestamp{}, errors.New("incompatible RTCP version")
 	}
 
 	if buf[1] != typeSenderReport {
-		return 0, 0, errors.New("RTCP packet is not of sender report type")
+		return NTPTimestamp{}, errors.New("RTCP packet is not of sender report type")
 	}
 
-	msw = binary.BigEndian.Uint32(buf[8:])
+	return NTPTimestamp{
-	lsw = binary.BigEndian.Uint32(buf[12:])
+		MSW: binary.BigEndian.Uint32(buf[8:]),
-
+		LSW: binary.BigEndian.Uint32(buf[12:]),
-	return
+	}, nil
 }

protocol/rtcp/parse_test.go

@@ -19,7 +19,7 @@ LICENSE
   It is distributed in the hope that it will be useful, but WITHOUT
   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
   FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
- 	for more details.
+  for more details.
 
   You should have received a copy of the GNU General Public License
   in gpl.txt.  If not, see http://www.gnu.org/licenses.
@@ -46,16 +46,16 @@ func TestTimestamp(t *testing.T) {
 		0x00, 0x01, 0xc2, 0xc5,
 	}
 
-	msw, lsw, err := Timestamp(report)
+	ts, err := Timestamp(report)
 	if err != nil {
 		t.Fatalf("did not expect error: %v", err)
 	}
 
-	if msw != expectedMSW {
+	if ts.MSW != expectedMSW {
-		t.Errorf("most significant word of timestamp is not what's expected. \nGot: %v\n Want: %v\n", msw, expectedMSW)
+		t.Errorf("most significant word of timestamp is not what's expected. \nGot: %v\n Want: %v\n", ts.MSW, expectedMSW)
 	}
 
-	if lsw != expectedLSW {
+	if ts.LSW != expectedLSW {
-		t.Errorf("least significant word of timestamp is not what's expected. \nGot: %v\n Want: %v\n", lsw, expectedLSW)
+		t.Errorf("least significant word of timestamp is not what's expected. \nGot: %v\n Want: %v\n", ts.LSW, expectedLSW)
 	}
 }