PLAY response has no RTP-Info header

[lattice0]

On my Vstarcam, I'm getting https://github.com/scottlamb/retina/blob/b1db9a9e8b94ff7077050cc26be0a50cbf1bd58e/src/client/parse.rs#L505, that is:

    let rtp_info = response
        .header(&rtsp_types::headers::RTP_INFO)
        .ok_or_else(|| "PLAY response has no RTP-Info header".to_string())?;

Does it have to have an RTP-Info header? According to https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rtsp/8432535c-ae1e-4d86-82de-29528cec1d3c,

This header is also sent by clients in the Play request when predictive stream selection is used. For more information on predictive stream selection, see section 2.2.6.10.5.

mine

PLAY rtsp://192.168.1.198:10554/tcp/av0_0 RTSP/1.0
Authorization: Digest username="admin", realm="RTSPD", nonce="kx2i6xqrqe44c86b667qy88lq1cl5670", uri="rtsp://192.168.1.198:10554/tcp/av0_0", response="a57bb4a903a2cddc2a40563b692da576"
CSeq: 4
Range: npt=0.000-
Session: 1632991860795359396
User-Agent: moonfire-rtsp test

RTSP/1.0 200 OK
Cseq: 4
Date: Mon, Jul 26 2021 19:14:55 GMT
Session: 1632991860795359396

yours

PLAY rtsp://192.168.1.198:10554/tcp/av0_0 RTSP/1.0
Authorization: Digest username="admin", algorithm="MD5", realm="RTSPD", nonce="03xah8kf4k0f228i77hp792zh70pd45q", uri="/tcp/av0_0", response="aa0fc4dbf695e0cb3be8cada1d1c7318"
CSeq: 5
Session: 4480630546592316416
User-Agent: RRTSP Client

RTSP/1.0 200 OK
Cseq: 5
Date: Mon, Jul 26 2021 19:17:16 GMT
Session: 4480630546592316416

[scottlamb]

RFC 2326 section 12 says it's required. But other cameras have left out some of the info it's supposed to include and I've made it work anyway. The bottom line is that if other clients work without this header, we can too, either by default or by passing a "quirks" option to be tolerant of non compliance.

[lattice0]

Indeed. I stopped developing my client after the play started working, I left out some of the RFC to read later while working on other parts of my app.

What you suggest we do? I can make the modifications if you want

[scottlamb]

The most important thing IMHO is to add this to the unit tests so we don't accidentally break it when changing something else.

As for the actual fix, I'm only skimming the code now, so I could be wrong, but think we can simply return Ok(()) from parse_play if the RTP-Info header is unset rather than trying to parse the header. We won't set the state.initial_seq, state.initial_rtptime, or state.ssrc for any of the streams then, but that might be fine:

• The caller can already tell us not to require initial_rtptime; see InitialTimestampPolicy. Audio and video may not be perfectly in sync without it, but this is probably the best we can do. (Although if other clients do better, I'd be interested in knowing how.)
• I don't think we ever require initial_seq or ssrc anyway. We just check that they're consistent if present.

[lattice0]

I did return Ok(()) and I got this on the first packet: thread 'retina_client_cam1' panicked at 'calledResult::unwrap()on anErrvalue: RtpPacketError { conn_ctx: ConnectionContext { local_addr: 172.17.0.2:41962, peer_addr: 192.168.1.198:10554, established_wall: WallTime(Timespec { sec: 1627337136, nsec: 400267155 }), established: Instant { tv_sec: 342610, tv_nsec: 775492850 } }, msg_ctx: RtspMessageContext { pos: 51429, received_wall: WallTime(Timespec { sec: 1627337139, nsec: 45641193 }), received: Instant { tv_sec: 342613, tv_nsec: 420867227 } }, channel_id: 0, stream_id: 0, ssrc: 2358, sequence_number: 38, description: "packet follows marked packet with same timestamp" }', /home/dev/orwell/liborwell/src/rtsp/retina_client.rs:149:61

which is from here https://github.com/scottlamb/retina/blob/main/src/codec/h264.rs#L181

it looks like the packet is being processed and then when std::mem::replace acts on it., however I'm not quite familiar with h264 nal units parsing yet.

Could this be related to the RTP-Info or is this another bug?

[scottlamb]

Looks like a separate issue to me.

[lattice0]

Do you want me to make this fix into a separate PR than the one from #9?

Currently it's like this for me:

pub(crate) fn parse_play(
    response: &rtsp_types::Response<Bytes>,
    presentation: &mut Presentation,
) -> Result<(), String> {
    // https://tools.ietf.org/html/rfc2326#section-12.33
    let rtp_info = match  response
    .header(&rtsp_types::headers::RTP_INFO) {
        Some(rtsp_info) => rtsp_info,
        None => return Ok(())
    };
    //...

from

pub(crate) fn parse_play(
    response: &rtsp_types::Response<Bytes>,
    presentation: &mut Presentation,
) -> Result<(), String> {
    // https://tools.ietf.org/html/rfc2326#section-12.33
    let rtp_info = response
        .header(&rtsp_types::headers::RTP_INFO)
        .ok_or_else(|| "PLAY response has no RTP-Info header".to_string())?;

https://github.com/scottlamb/retina/blob/b1db9a9e8b94ff7077050cc26be0a50cbf1bd58e/src/client/parse.rs#L500

[scottlamb]

One PR with one commit per problem would be fine with me. Less annoying IMHO than managing separate PRs and having to state "this PR depends on that one, which depends on that one", or keeping them independent and then having to rebase.