Allow RTCP packets for keepalive also in state PLAY

[nzottmann]

I would like to use https://github.com/aler9/rtsp-simple-server with a rtsp client which sends RTCP Receiver Reports every few seconds, but no RTSP keepalives like GET_PARAMETER in state PLAY. Thus, rtsp-simple-server terminates the connection every 60s and the client starts a new one, resulting in an interrupted stream every minute.

This is expected as mentioned in https://github.com/aler9/rtsp-simple-server/issues/647#issuecomment-950139904 and in https://github.com/aler9/gortsplib/blob/6deba3e4546d858878a3a88ae4c328778ecd3bb3/server_session.go#L415-L425

As I am not able to change my client, I need gortsplib to accept RTCP Receiver Reports also in state PLAY. In RFC 2326 I found

If the server is in state Playing or Recording and in unicast mode, it MAY revert to Init and tear down the RTSP session if it has not received "wellness" information, such as RTCP reports or RTSP commands, from the client for a defined interval, with a default of one minute.

• Is there a reason why gortsplib does not accept RTCP packets for keepalive in PLAY?
• Would you accept a pull request for accepting both ways of keepalive in PLAY and RECORD?

If there is a reason for the current implementation and you would like to keep it this way, I would be fine with doing this just for me in a fork, but I wanted to ask if this is something you would also view as useful.

[nzottmann]

I referenced the outdated RF 2326, the current RFC 7826 has more information on showing liveness:

To show liveness between RTSP requests being issued
   to accomplish other things, the following mechanisms can be used, in
   descending order of preference:

   RTCP: [...]
   SET_PARAMETER:  [...]
   GET_PARAMETER:  [...]
   OPTIONS:  [...]

In my opinion there is nothing against accepting RTCP packets for PLAY as well.

[aler9]

Hello, i've checked the code and the specification, and you're right, this is a missing feature.

this line

} else if now.Sub(ss.lastRequestTime) >= ss.s.sessionTimeout {

can be changed into

lft := atomic.LoadInt64(ss.udpLastPacketTime)
...
} else if now.Sub(ss.lastRequestTime) >= ss.s.sessionTimeout && now.Sub(time.Unix(lft, 0)) >= ss.s.ReadTimeout {

while this line

atomic.StoreInt64(sm.ss.udpLastPacketTime, now.Unix())

must be added to serverSessionMedia.readRTCPUDPPlay.

A PR is welcome.

[nzottmann]

Thank you for the fast reaction and the suggestions! I submitted PR #186 implementing the changes. I checked that they solve described problem with my client. The tests are also passing.

[aler9]

fixed by #186

[aler9]

added in v0.21.3

[github-actions[bot]]

This issue is being locked automatically because it has been closed for more than 6 months. Please open a new issue in case you encounter a similar problem.