Repository: bluenviron/gortsplib
Pull request #758 (2025-04-15 15:12:05)

Allow setting response body in OnDescribe
Since commit 8c6495c33b3194f1bf34f3f8a87f51907e567915, the server can no longer directly specify the response body to DESCRIBE requests. This is a breaking change and is a limitation for users who manually handle their SDPs. This commit allows setting the body explicitly and takes it into account if the `ServerStream` part of the `OnDecsribe` return value is nil. If neither the stream, nor the body are set, the application panics.

@aler9 if you approve this change, I can amend the tests.

aler9: Hello, yes, you're right, but if you are interested into keeping this feature working, add a dedicated unit test too so things cannot break next time. Thanks.
ygabuev: Hey @aler9, thanks for your prompt response. I've added a test, please take a look if it makes sense. If you decide to merge the PR, a new bugfix release would be very appreciated :pray: 

Patch:
diff --git a/pkg/format/rtph264/decoder.go b/pkg/format/rtph264/decoder.go
index 519c045d..725ecd5a 100644
--- a/pkg/format/rtph264/decoder.go
+++ b/pkg/format/rtph264/decoder.go
@@ -59,9 +59,10 @@ type Decoder struct {
 	annexBMode          bool
 
 	// for Decode()
-	frameBuffer     [][]byte
-	frameBufferLen  int
-	frameBufferSize int
+	frameBuffer          [][]byte
+	frameBufferLen       int
+	frameBufferSize      int
+	frameBufferTimestamp uint32
 }
 
 // Init initializes the decoder.
@@ -222,12 +223,46 @@ func (d *Decoder) Decode(pkt *rtp.Packet) ([][]byte, error) {
 	}
 	l := len(nalus)
 
+	// support splitting access units by timestamp.
+	// (some cameras do not use the Marker field, like the FLIR M400)
+	if d.frameBuffer != nil && pkt.Timestamp != d.frameBufferTimestamp {
+		ret := d.frameBuffer
+		d.resetFrameBuffer()
+
+		err = d.addToFrameBuffer(nalus, l, pkt.Timestamp)
+		if err != nil {
+			return nil, err
+		}
+
+		return ret, nil
+	}
+
+	err = d.addToFrameBuffer(nalus, l, pkt.Timestamp)
+	if err != nil {
+		return nil, err
+	}
+
+	if !pkt.Marker {
+		return nil, ErrMorePacketsNeeded
+	}
+
+	ret := d.frameBuffer
+	d.resetFrameBuffer()
+
+	return ret, nil
+}
+
+func (d *Decoder) resetFrameBuffer() {
+	d.frameBuffer = nil // do not reuse frameBuffer to avoid race conditions
+	d.frameBufferLen = 0
+	d.frameBufferSize = 0
+}
+
+func (d *Decoder) addToFrameBuffer(nalus [][]byte, l int, ts uint32) error {
 	if (d.frameBufferLen + l) > h264.MaxNALUsPerAccessUnit {
 		errCount := d.frameBufferLen + l
-		d.frameBuffer = nil
-		d.frameBufferLen = 0
-		d.frameBufferSize = 0
-		return nil, fmt.Errorf("NALU count (%d) exceeds maximum allowed (%d)",
+		d.resetFrameBuffer()
+		return fmt.Errorf("NALU count (%d) exceeds maximum allowed (%d)",
 			errCount, h264.MaxNALUsPerAccessUnit)
 	}
 
@@ -235,29 +270,16 @@ func (d *Decoder) Decode(pkt *rtp.Packet) ([][]byte, error) {
 
 	if (d.frameBufferSize + addSize) > h264.MaxAccessUnitSize {
 		errSize := d.frameBufferSize + addSize
-		d.frameBuffer = nil
-		d.frameBufferLen = 0
-		d.frameBufferSize = 0
-		return nil, fmt.Errorf("access unit size (%d) is too big, maximum is %d",
+		d.resetFrameBuffer()
+		return fmt.Errorf("access unit size (%d) is too big, maximum is %d",
 			errSize, h264.MaxAccessUnitSize)
 	}
 
 	d.frameBuffer = append(d.frameBuffer, nalus...)
 	d.frameBufferLen += l
 	d.frameBufferSize += addSize
-
-	if !pkt.Marker {
-		return nil, ErrMorePacketsNeeded
-	}
-
-	ret := d.frameBuffer
-
-	// do not reuse frameBuffer to avoid race conditions
-	d.frameBuffer = nil
-	d.frameBufferLen = 0
-	d.frameBufferSize = 0
-
-	return ret, nil
+	d.frameBufferTimestamp = ts
+	return nil
 }
 
 // some cameras / servers wrap NALUs into Annex-B
diff --git a/server_conn.go b/server_conn.go
index 38a8ed54..50f24d85 100644
--- a/server_conn.go
+++ b/server_conn.go
@@ -328,8 +328,8 @@ func (sc *ServerConn) handleRequestInner(req *base.Request) (*base.Response, err
 			})
 
 			if res.StatusCode == base.StatusOK {
-				if stream == nil {
-					panic("stream cannot be nil when StatusCode is StatusOK")
+				if stream == nil && len(res.Body) == 0 {
+					panic("stream should be not nil or response body should be nonempty when StatusCode is StatusOK")
 				}
 
 				if res.Header == nil {
@@ -339,6 +339,10 @@ func (sc *ServerConn) handleRequestInner(req *base.Request) (*base.Response, err
 				res.Header["Content-Base"] = base.HeaderValue{req.URL.String() + "/"}
 				res.Header["Content-Type"] = base.HeaderValue{"application/sdp"}
 
+				if stream == nil {
+					return res, err
+				}
+
 				// VLC uses multicast if the SDP contains a multicast address.
 				// therefore, we introduce a special query (vlcmulticast) that allows
 				// to return a SDP that contains a multicast address.


Test patch:
diff --git a/pkg/format/rtph264/decoder_test.go b/pkg/format/rtph264/decoder_test.go
index 93a79016..25ad7748 100644
--- a/pkg/format/rtph264/decoder_test.go
+++ b/pkg/format/rtph264/decoder_test.go
@@ -189,37 +189,86 @@ func TestDecodeAnnexB(t *testing.T) {
 }
 
 func TestDecodeAccessUnit(t *testing.T) {
-	d := &Decoder{}
-	err := d.Init()
-	require.NoError(t, err)
-
-	nalus, err := d.Decode(&rtp.Packet{
-		Header: rtp.Header{
-			Version:        2,
-			Marker:         false,
-			PayloadType:    96,
-			SequenceNumber: 17647,
-			Timestamp:      2289531307,
-			SSRC:           0x9dbb7812,
+	for _, ca := range []struct {
+		name string
+		pkts []*rtp.Packet
+		au   [][]byte
+	}{
+		{
+			"marker-splitted",
+			[]*rtp.Packet{
+				{
+					Header: rtp.Header{
+						Version:        2,
+						Marker:         false,
+						PayloadType:    96,
+						SequenceNumber: 17647,
+						Timestamp:      2289531307,
+						SSRC:           0x9dbb7812,
+					},
+					Payload: []byte{1, 2},
+				},
+				{
+					Header: rtp.Header{
+						Version:        2,
+						Marker:         true,
+						PayloadType:    96,
+						SequenceNumber: 17647,
+						Timestamp:      2289531307,
+						SSRC:           0x9dbb7812,
+					},
+					Payload: []byte{3, 4},
+				},
+			},
+			[][]byte{{1, 2}, {3, 4}},
 		},
-		Payload: []byte{0x01, 0x02},
-	})
-	require.Equal(t, ErrMorePacketsNeeded, err)
-	require.Equal(t, [][]byte(nil), nalus)
-
-	nalus, err = d.Decode(&rtp.Packet{
-		Header: rtp.Header{
-			Version:        2,
-			Marker:         true,
-			PayloadType:    96,
-			SequenceNumber: 17647,
-			Timestamp:      2289531307,
-			SSRC:           0x9dbb7812,
+		{
+			"timestamp-splitted (FLIR M400)",
+			[]*rtp.Packet{
+				{
+					Header: rtp.Header{
+						Version:        2,
+						Marker:         false,
+						PayloadType:    96,
+						SequenceNumber: 17647,
+						Timestamp:      2289531307,
+						SSRC:           0x9dbb7812,
+					},
+					Payload: []byte{1, 2},
+				},
+				{
+					Header: rtp.Header{
+						Version:        2,
+						Marker:         false,
+						PayloadType:    96,
+						SequenceNumber: 17647,
+						Timestamp:      2289531308,
+						SSRC:           0x9dbb7812,
+					},
+					Payload: []byte{3, 4},
+				},
+			},
+			[][]byte{{1, 2}},
 		},
-		Payload: []byte{0x01, 0x02},
-	})
-	require.NoError(t, err)
-	require.Equal(t, [][]byte{{0x01, 0x02}, {0x01, 0x02}}, nalus)
+	} {
+		t.Run(ca.name, func(t *testing.T) {
+			d := &Decoder{}
+			err := d.Init()
+			require.NoError(t, err)
+
+			var au [][]byte
+
+			for i, pkt := range ca.pkts {
+				au, err = d.Decode(pkt)
+				if i != len(ca.pkts)-1 {
+					require.Equal(t, ErrMorePacketsNeeded, err)
+				} else {
+					require.NoError(t, err)
+					require.Equal(t, ca.au, au)
+				}
+			}
+		})
+	}
 }
 
 func TestDecoderErrorNALUSize(t *testing.T) {
diff --git a/server_test.go b/server_test.go
index 4a5af334..f02ac861 100644
--- a/server_test.go
+++ b/server_test.go
@@ -354,6 +354,41 @@ func TestServerErrorNilURL(t *testing.T) {
 	<-nconnClosed
 }
 
+func TestServerDescribeNonNilBody(t *testing.T) {
+	sdpBody := []byte("foo-sdp")
+
+	s := &Server{
+		Handler: &testServerHandler{
+			onDescribe: func(_ *ServerHandlerOnDescribeCtx) (*base.Response, *ServerStream, error) {
+				return &base.Response{
+					StatusCode: base.StatusOK,
+					Body:       sdpBody,
+				}, nil, nil
+			},
+		},
+		RTSPAddress: "localhost:8554",
+	}
+	err := s.Start()
+	require.NoError(t, err)
+	defer s.Close()
+
+	nconn, err := net.Dial("tcp", "localhost:8554")
+	require.NoError(t, err)
+	defer nconn.Close()
+	conn := conn.NewConn(nconn)
+
+	res, err := writeReqReadRes(conn, base.Request{
+		Method: base.Describe,
+		URL:    mustParseURL("rtsp://localhost:8554/"),
+		Header: base.Header{
+			"CSeq": base.HeaderValue{"1"},
+		},
+	})
+	require.NoError(t, err)
+	require.Equal(t, base.StatusOK, res.StatusCode)
+	require.Equal(t, sdpBody, res.Body)
+}
+
 type testServerErrMethodNotImplemented struct {
 	stream *ServerStream
 }