Repository: bluenviron/gortsplib
Pull request #208 (2023-03-19 16:55:35)

client: replace OnWarning with Log

codecov-commenter: ## [Codecov](https://codecov.io/gh/bluenviron/gortsplib/pull/208?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#208](https://codecov.io/gh/bluenviron/gortsplib/pull/208?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (b8f3894) into [main](https://codecov.io/gh/bluenviron/gortsplib/commit/1005b38ec53d3b64f6e8b8d090f04a2929e33685?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (1005b38) will **decrease** coverage by `0.08%`.
> The diff coverage is `71.42%`.

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

```diff
@@            Coverage Diff             @@
##             main     #208      +/-   ##
==========================================
- Coverage   86.90%   86.82%   -0.08%     
==========================================
  Files         102      102              
  Lines       10697    10704       +7     
==========================================
- Hits         9296     9294       -2     
- Misses       1079     1086       +7     
- Partials      322      324       +2     
```


| [Impacted Files](https://codecov.io/gh/bluenviron/gortsplib/pull/208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [client.go](https://codecov.io/gh/bluenviron/gortsplib/pull/208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y2xpZW50Lmdv) | `74.05% <50.00%> (-0.57%)` | :arrow_down: |
| [client\_media.go](https://codecov.io/gh/bluenviron/gortsplib/pull/208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y2xpZW50X21lZGlhLmdv) | `88.11% <92.30%> (ø)` | |
| [client\_format.go](https://codecov.io/gh/bluenviron/gortsplib/pull/208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y2xpZW50X2Zvcm1hdC5nbw==) | `95.38% <100.00%> (ø)` | |

... and [1 file with indirect coverage changes](https://codecov.io/gh/bluenviron/gortsplib/pull/208/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)


Patch:
diff --git a/client.go b/client.go
index 7010126e..7e58a59d 100644
--- a/client.go
+++ b/client.go
@@ -10,6 +10,7 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"log"
 	"net"
 	"strconv"
 	"strings"
@@ -140,6 +141,24 @@ type clientRes struct {
 	err     error
 }
 
+// LogLevel is a log level.
+type LogLevel int
+
+// Log levels.
+const (
+	LogLevelDebug LogLevel = iota + 1
+	LogLevelInfo
+	LogLevelWarn
+	LogLevelError
+)
+
+// LogFunc is the prototype of the log function.
+type LogFunc func(level LogLevel, format string, args ...interface{})
+
+func defaultLog(level LogLevel, format string, args ...interface{}) {
+	log.Printf(format, args...)
+}
+
 // Client is a RTSP client.
 type Client struct {
 	//
@@ -206,11 +225,18 @@ type Client struct {
 	OnRequest func(*base.Request)
 	// called after every response.
 	OnResponse func(*base.Response)
-	// called when there's a non-fatal warning.
+	// Deprecated. replaced by Log.
 	OnWarning func(error)
-	// Deprecated: replaced by OnWarning.
+	// Deprecated. replaced by Log.
 	OnDecodeError func(error)
 
+	//
+	// logging (all optional)
+	//
+	// function that receives log messages.
+	// It defaults to log.Printf.
+	Log LogFunc
+
 	//
 	// private
 	//
@@ -313,12 +339,19 @@ func (c *Client) Start(scheme string, host string) error {
 		c.OnResponse = func(*base.Response) {
 		}
 	}
-	if c.OnWarning == nil {
-		c.OnWarning = func(error) {
+	if c.OnDecodeError != nil {
+		c.Log = func(level LogLevel, format string, args ...interface{}) {
+			c.OnDecodeError(fmt.Errorf(format, args...))
 		}
 	}
-	if c.OnDecodeError != nil {
-		c.OnWarning = c.OnDecodeError
+	if c.OnWarning != nil {
+		c.Log = func(level LogLevel, format string, args ...interface{}) {
+			c.OnDecodeError(fmt.Errorf(format, args...))
+		}
+	}
+
+	if c.Log == nil {
+		c.Log = defaultLog
 	}
 
 	// private
@@ -591,7 +624,7 @@ func (c *Client) checkState(allowed map[clientState]struct{}) error {
 }
 
 func (c *Client) trySwitchingProtocol() error {
-	c.OnWarning(fmt.Errorf("no UDP packets received, switching to TCP"))
+	c.Log(LogLevelWarn, "no UDP packets received, switching to TCP")
 
 	prevScheme := c.scheme
 	prevHost := c.host
@@ -632,7 +665,7 @@ func (c *Client) trySwitchingProtocol() error {
 }
 
 func (c *Client) trySwitchingProtocol2(medi *media.Media, baseURL *url.URL) (*base.Response, error) {
-	c.OnWarning(fmt.Errorf("switching to TCP because server requested it"))
+	c.Log(LogLevelWarn, "switching to TCP because server requested it")
 
 	prevScheme := c.scheme
 	prevHost := c.host
@@ -1247,7 +1280,7 @@ func (c *Client) doSetup(
 		if res.StatusCode == base.StatusUnsupportedTransport &&
 			c.effectiveTransport == nil &&
 			c.Transport == nil {
-			c.OnWarning(fmt.Errorf("switching to TCP because server requested it"))
+			c.Log(LogLevelWarn, "switching to TCP because server requested it")
 			v := TransportTCP
 			c.effectiveTransport = &v
 			return c.doSetup(medi, baseURL, 0, 0)
diff --git a/client_format.go b/client_format.go
index 99d87d08..35c7691a 100644
--- a/client_format.go
+++ b/client_format.go
@@ -1,7 +1,6 @@
 package gortsplib
 
 import (
-	"fmt"
 	"time"
 
 	"github.com/pion/rtcp"
@@ -95,7 +94,7 @@ func (ct *clientFormat) writePacketRTPWithNTP(pkt *rtp.Packet, ntp time.Time) er
 func (ct *clientFormat) readRTPUDP(pkt *rtp.Packet) {
 	packets, missing := ct.udpReorderer.Process(pkt)
 	if missing != 0 {
-		ct.c.OnWarning(fmt.Errorf("%d RTP packet(s) lost", missing))
+		ct.c.Log(LogLevelWarn, "%d RTP packet(s) lost", missing)
 		// do not return
 	}
 
diff --git a/client_media.go b/client_media.go
index d754979a..a5e538fc 100644
--- a/client_media.go
+++ b/client_media.go
@@ -1,7 +1,6 @@
 package gortsplib
 
 import (
-	"fmt"
 	"sync/atomic"
 	"time"
 
@@ -222,14 +221,14 @@ func (cm *clientMedia) readRTCPTCPPlay(payload []byte) error {
 	atomic.StoreInt64(cm.c.tcpLastFrameTime, now.Unix())
 
 	if len(payload) > maxPacketSize {
-		cm.c.OnWarning(fmt.Errorf("RTCP packet size (%d) is greater than maximum allowed (%d)",
-			len(payload), maxPacketSize))
+		cm.c.Log(LogLevelWarn, "RTCP packet size (%d) is greater than maximum allowed (%d)",
+			len(payload), maxPacketSize)
 		return nil
 	}
 
 	packets, err := rtcp.Unmarshal(payload)
 	if err != nil {
-		cm.c.OnWarning(err)
+		cm.c.Log(LogLevelWarn, "%v", err)
 		return nil
 	}
 
@@ -246,14 +245,14 @@ func (cm *clientMedia) readRTPTCPRecord(payload []byte) error {
 
 func (cm *clientMedia) readRTCPTCPRecord(payload []byte) error {
 	if len(payload) > maxPacketSize {
-		cm.c.OnWarning(fmt.Errorf("RTCP packet size (%d) is greater than maximum allowed (%d)",
-			len(payload), maxPacketSize))
+		cm.c.Log(LogLevelWarn, "RTCP packet size (%d) is greater than maximum allowed (%d)",
+			len(payload), maxPacketSize)
 		return nil
 	}
 
 	packets, err := rtcp.Unmarshal(payload)
 	if err != nil {
-		cm.c.OnWarning(err)
+		cm.c.Log(LogLevelWarn, "%v", err)
 		return nil
 	}
 
@@ -270,20 +269,20 @@ func (cm *clientMedia) readRTPUDPPlay(payload []byte) error {
 	atomic.AddUint64(cm.c.BytesReceived, uint64(plen))
 
 	if plen == (maxPacketSize + 1) {
-		cm.c.OnWarning(fmt.Errorf("RTP packet is too big to be read with UDP"))
+		cm.c.Log(LogLevelWarn, "RTP packet is too big to be read with UDP")
 		return nil
 	}
 
 	pkt := &rtp.Packet{}
 	err := pkt.Unmarshal(payload)
 	if err != nil {
-		cm.c.OnWarning(err)
+		cm.c.Log(LogLevelWarn, "%v", err)
 		return nil
 	}
 
 	forma, ok := cm.formats[pkt.PayloadType]
 	if !ok {
-		cm.c.OnWarning(fmt.Errorf("received RTP packet with unknown payload type (%d)", pkt.PayloadType))
+		cm.c.Log(LogLevelWarn, "received RTP packet with unknown payload type (%d)", pkt.PayloadType)
 		return nil
 	}
 
@@ -298,13 +297,13 @@ func (cm *clientMedia) readRTCPUDPPlay(payload []byte) error {
 	atomic.AddUint64(cm.c.BytesReceived, uint64(plen))
 
 	if plen == (maxPacketSize + 1) {
-		cm.c.OnWarning(fmt.Errorf("RTCP packet is too big to be read with UDP"))
+		cm.c.Log(LogLevelWarn, "RTCP packet is too big to be read with UDP")
 		return nil
 	}
 
 	packets, err := rtcp.Unmarshal(payload)
 	if err != nil {
-		cm.c.OnWarning(err)
+		cm.c.Log(LogLevelWarn, "%v", err)
 		return nil
 	}
 
@@ -332,13 +331,13 @@ func (cm *clientMedia) readRTCPUDPRecord(payload []byte) error {
 	atomic.AddUint64(cm.c.BytesReceived, uint64(plen))
 
 	if plen == (maxPacketSize + 1) {
-		cm.c.OnWarning(fmt.Errorf("RTCP packet is too big to be read with UDP"))
+		cm.c.Log(LogLevelWarn, "RTCP packet is too big to be read with UDP")
 		return nil
 	}
 
 	packets, err := rtcp.Unmarshal(payload)
 	if err != nil {
-		cm.c.OnWarning(err)
+		cm.c.Log(LogLevelWarn, "%v", err)
 		return nil
 	}
 


Test patch:
diff --git a/client_play_test.go b/client_play_test.go
index d992331c..34656096 100644
--- a/client_play_test.go
+++ b/client_play_test.go
@@ -3,6 +3,7 @@ package gortsplib
 import (
 	"bytes"
 	"crypto/tls"
+	"fmt"
 	"net"
 	"strconv"
 	"strings"
@@ -1049,8 +1050,8 @@ func TestClientPlayAutomaticProtocol(t *testing.T) {
 		packetRecv := make(chan struct{})
 
 		c := Client{
-			OnWarning: func(err error) {
-				require.EqualError(t, err, "switching to TCP because server requested it")
+			Log: func(level LogLevel, format string, args ...interface{}) {
+				require.Equal(t, format, "switching to TCP because server requested it")
 			},
 		}
 		err = readAll(&c, "rtsp://localhost:8554/teststream",
@@ -1225,8 +1226,8 @@ func TestClientPlayAutomaticProtocol(t *testing.T) {
 		packetRecv := make(chan struct{})
 
 		c := Client{
-			OnWarning: func(err error) {
-				require.EqualError(t, err, "switching to TCP because server requested it")
+			Log: func(level LogLevel, format string, args ...interface{}) {
+				require.Equal(t, format, "switching to TCP because server requested it")
 			},
 		}
 		err = readAll(&c, "rtsp://localhost:8554/teststream",
@@ -1459,8 +1460,8 @@ func TestClientPlayAutomaticProtocol(t *testing.T) {
 		packetRecv := make(chan struct{})
 
 		c := Client{
-			OnWarning: func(err error) {
-				require.EqualError(t, err, "no UDP packets received, switching to TCP")
+			Log: func(level LogLevel, format string, args ...interface{}) {
+				require.Equal(t, format, "no UDP packets received, switching to TCP")
 			},
 			ReadTimeout: 1 * time.Second,
 		}
@@ -3072,7 +3073,8 @@ func TestClientPlayDecodeErrors(t *testing.T) {
 					v := TransportTCP
 					return &v
 				}(),
-				OnWarning: func(err error) {
+				Log: func(level LogLevel, format string, args ...interface{}) {
+					err := fmt.Errorf(format, args...)
 					switch {
 					case ca.proto == "udp" && ca.name == "rtp invalid":
 						require.EqualError(t, err, "RTP header size insufficient: 2 < 4")
diff --git a/client_record_test.go b/client_record_test.go
index 10fa98e7..c9d798b1 100644
--- a/client_record_test.go
+++ b/client_record_test.go
@@ -3,6 +3,7 @@ package gortsplib
 import (
 	"bytes"
 	"crypto/tls"
+	"fmt"
 	"net"
 	"strings"
 	"testing"
@@ -1021,7 +1022,8 @@ func TestClientRecordDecodeErrors(t *testing.T) {
 					v := TransportTCP
 					return &v
 				}(),
-				OnWarning: func(err error) {
+				Log: func(level LogLevel, format string, args ...interface{}) {
+					err := fmt.Errorf(format, args...)
 					switch {
 					case ca.proto == "udp" && ca.name == "rtcp invalid":
 						require.EqualError(t, err, "rtcp: packet too short")