Repository: pion/webrtc
Pull request #1437 (2020-09-21 14:05:37)

Implements GetStats: RTCCodecStats 
implemented getstats api for rpccodec

#### Description

#### Reference issue
https://github.com/pion/webrtc/issues/610
Fixes #...


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/1437?src=pr&el=h1) Report
> Merging [#1437](https://codecov.io/gh/pion/webrtc/pull/1437?src=pr&el=desc) into [master](https://codecov.io/gh/pion/webrtc/commit/062e55d7693a8b77c9c7fe47c695c505d7e294ce?el=desc) will **decrease** coverage by `2.24%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pion/webrtc/pull/1437/graphs/tree.svg?width=650&height=150&src=pr&token=XOOuQo07I2)](https://codecov.io/gh/pion/webrtc/pull/1437?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1437      +/-   ##
==========================================
- Coverage   73.94%   71.70%   -2.25%     
==========================================
  Files          73       53      -20     
  Lines        5197     1947    -3250     
==========================================
- Hits         3843     1396    -2447     
+ Misses        999      453     -546     
+ Partials      355       98     -257     
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `?` | |
| #wasm | `71.70% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/1437?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [icetcp.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-aWNldGNwLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-52.18%)` | :arrow_down: |
| [gathering\_complete\_promise.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-Z2F0aGVyaW5nX2NvbXBsZXRlX3Byb21pc2UuZ28=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [dtlsrole.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-ZHRsc3JvbGUuZ28=) | `68.96% <0.00%> (-31.04%)` | :arrow_down: |
| [networktype.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-bmV0d29ya3R5cGUuZ28=) | `46.80% <0.00%> (-19.15%)` | :arrow_down: |
| [icegathererstate.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-aWNlZ2F0aGVyZXJzdGF0ZS5nbw==) | `84.61% <0.00%> (-15.39%)` | :arrow_down: |
| [icecandidatetype.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRldHlwZS5nbw==) | `61.11% <0.00%> (-13.89%)` | :arrow_down: |
| [operations.go](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree#diff-b3BlcmF0aW9ucy5nbw==) | `83.33% <0.00%> (-11.12%)` | :arrow_down: |
| ... and [20 more](https://codecov.io/gh/pion/webrtc/pull/1437/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/1437?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/webrtc/pull/1437?src=pr&el=footer). Last update [062e55d...ee84593](https://codecov.io/gh/pion/webrtc/pull/1437?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Sean-Der: Look pretty good @obasajujoshua31 I am still reviewing but would you mind changing the commit message to something like this.

```
Add GetMediaStats to StatsReport

Add an API to get basic stats around media. 

Relates to #610 
```

If I am reading `git log` the current commit message makes it really hard to understand what is going on. thanks!

Patch:
diff --git a/mediaengine.go b/mediaengine.go
index ed8974dc..1f681f69 100644
--- a/mediaengine.go
+++ b/mediaengine.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/pion/rtp"
 	"github.com/pion/rtp/codecs"
@@ -340,6 +341,7 @@ type RTPCodec struct {
 	Name        string
 	PayloadType uint8
 	Payloader   rtp.Payloader
+	statsID     string
 }
 
 // NewRTPCodec is used to define a new codec
@@ -363,6 +365,7 @@ func NewRTPCodec(
 		Payloader:   payloader,
 		Type:        codecType,
 		Name:        name,
+		statsID:     fmt.Sprintf("RTPCodec-%d", time.Now().UnixNano()),
 	}
 }
 
@@ -411,3 +414,22 @@ type RTPCapabilities struct {
 	Codecs           []RTPCodecCapability
 	HeaderExtensions []RTPHeaderExtensionCapability
 }
+
+func (m *MediaEngine) collectStats(collector *statsReportCollector) {
+
+	for _, codec := range m.codecs {
+		collector.Collecting()
+		stats := CodecStats{
+			Timestamp:   statsTimestampFrom(time.Now()),
+			Type:        StatsTypeCodec,
+			ID:          codec.statsID,
+			PayloadType: codec.PayloadType,
+			MimeType:    codec.MimeType,
+			ClockRate:   codec.ClockRate,
+			Channels:    uint8(codec.Channels),
+			SDPFmtpLine: codec.SDPFmtpLine,
+		}
+
+		collector.Collect(stats.ID, stats)
+	}
+}
diff --git a/peerconnection.go b/peerconnection.go
index a4041fe6..0277d35f 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -1958,6 +1958,8 @@ func (pc *PeerConnection) GetStats() StatsReport {
 	}
 	pc.mu.Unlock()
 
+	pc.api.mediaEngine.collectStats(statsCollector)
+
 	return statsCollector.Ready()
 }
 
diff --git a/stats.go b/stats.go
index b47d9149..e53248d2 100644
--- a/stats.go
+++ b/stats.go
@@ -153,7 +153,7 @@ type CodecStats struct {
 	ID string `json:"id"`
 
 	// PayloadType as used in RTP encoding or decoding
-	PayloadType uint32 `json:"payloadType"`
+	PayloadType uint8 `json:"payloadType"`
 
 	// CodecType of this CodecStats
 	CodecType CodecType `json:"codecType"`
@@ -169,7 +169,7 @@ type CodecStats struct {
 	ClockRate uint32 `json:"clockRate"`
 
 	// Channels is 2 for stereo, missing for most other cases.
-	Channels uint32 `json:"channels"`
+	Channels uint8 `json:"channels"`
 
 	// SDPFmtpLine is the a=fmtp line in the SDP corresponding to the codec,
 	// i.e., after the colon following the PT.
diff --git a/stats_go.go b/stats_go.go
index f1b783e7..d6fc06ae 100644
--- a/stats_go.go
+++ b/stats_go.go
@@ -75,4 +75,20 @@ func (r StatsReport) GetCertificateStats(c *Certificate) (CertificateStats, bool
 		return CertificateStats{}, false
 	}
 	return certificateStats, true
+
+}
+
+// GetCodecStats is a helper method to return the associated stats for a given Codec
+func (r StatsReport) GetCodecStats(c *RTPCodec) (CodecStats, bool) {
+	statsID := c.statsID
+	stats, ok := r[statsID]
+	if !ok {
+		return CodecStats{}, false
+	}
+
+	codecStats, ok := stats.(CodecStats)
+	if !ok {
+		return CodecStats{}, false
+	}
+	return codecStats, true
 }


Test patch:
diff --git a/stats_go_test.go b/stats_go_test.go
index 9f36298d..b9cb6bd3 100644
--- a/stats_go_test.go
+++ b/stats_go_test.go
@@ -5,6 +5,8 @@ package webrtc
 import (
 	"encoding/json"
 	"fmt"
+	"github.com/stretchr/testify/require"
+	"math/rand"
 	"sync"
 	"testing"
 	"time"
@@ -98,6 +100,13 @@ func getDataChannelStats(t *testing.T, report StatsReport, dc *DataChannel) Data
 	return stats
 }
 
+func getCodecStats(t *testing.T, report StatsReport, c *RTPCodec) CodecStats {
+	stats, ok := report.GetCodecStats(c)
+	assert.True(t, ok)
+	assert.Equal(t, stats.Type, StatsTypeCodec)
+	return stats
+}
+
 func getTransportStats(t *testing.T, report StatsReport, statsID string) TransportStats {
 	stats, ok := report[statsID]
 	assert.True(t, ok)
@@ -194,6 +203,12 @@ func TestPeerConnection_GetStats(t *testing.T) {
 	offerPC, answerPC, err := newPair()
 	assert.NoError(t, err)
 
+	track1, err := offerPC.NewTrack(DefaultPayloadTypeVP8, rand.Uint32(), "video", "pion1")
+	require.NoError(t, err)
+
+	_, err = offerPC.AddTrack(track1)
+	require.NoError(t, err)
+
 	baseLineReportPCOffer := offerPC.GetStats()
 	baseLineReportPCAnswer := answerPC.GetStats()
 
@@ -262,6 +277,11 @@ func TestPeerConnection_GetStats(t *testing.T) {
 	assert.NotEmpty(t, findLocalCandidateStats(reportPCAnswer))
 	assert.NotEmpty(t, findRemoteCandidateStats(reportPCAnswer))
 	assert.NotEmpty(t, findCandidatePairStats(t, reportPCAnswer))
+	assert.NoError(t, err)
+	for _, codec := range offerPC.api.mediaEngine.codecs {
+		codecStat := getCodecStats(t, reportPCOffer, codec)
+		assert.NotEmpty(t, codecStat)
+	}
 
 	// Close answer DC now
 	dcWait = sync.WaitGroup{}