Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

permissive parsing of a= line between session's c= and t= #10

Closed
scottlamb opened this issue Aug 23, 2021 · 3 comments
Closed

permissive parsing of a= line between session's c= and t= #10

scottlamb opened this issue Aug 23, 2021 · 3 comments

Comments

@scottlamb
Copy link
Contributor

A Retina user reported an error trying to connect to an Anpviz IPC-D250 camera. This SDP wouldn't parse:

v=0
o=- 1109162014219182 1109162014219192 IN IP4 x.y.z.w
s=RTSP/RTP stream from anjvision ipcamera
e=NONE
c=IN IP4 0.0.0.0
a=tool:LIVE555 Streaming Media v2011.05.25 CHAM.LI@ANJVISION.COM
t=0 0
a=range:npt=0-
a=control:*
m=video 0 RTP/AVP 96
a=rtpmap:96 H264/90000
a=control:trackID=1
a=fmtp:96 profile-level-id=4D401F;packetization-mode=0;sprop-parameter-sets=Z01AH5WgLASabAQ=,aO48gA==;config=00000001674d401f95a02c049a6c040000000168ee3c800000000106f02c0445c6f5000620ebc2f3f7639e48250bfcb561bb2b85dda6fe5f06cc8b887b6a915f5aa3bebfffffffffff7380
a=x-dimensions: 704, 576
a=x-framerate: 12
m=audio 0 RTP/AVP 0
a=rtpmap:0 MPEG4-GENERIC/16000/2
a=fmtp:0 config=1408
a=control:trackID=2
a=Media_header:MEDIAINFO=494D4B48010100000400010010710110401F000000FA000000000000000000000000000000000000;
a=appversion:1.0

Apparently it contradicts RFC 8866 section 9 to have an a= between the c= and the t=:

   ; SDP Syntax
   session-description = version-field
                         origin-field
                         session-name-field
                         [information-field]
                         [uri-field]
                         *email-field
                         *phone-field
                         [connection-field]
                         *bandwidth-field
                         1*time-description
                         [key-field]
                         *attribute-field
                         *media-description

Are you open to sdp-types parsing this anyway? Either by default or with a permissive flag (I don't care which).

More context at scottlamb/retina#26 and scottlamb/moonfire-nvr#151.

@sdroege
Copy link
Owner

sdroege commented Aug 24, 2021

That's annoying, but doing more permissive parsing here seems fine. SDP is an old standard and there's a lot of broken legacy software and hardware out there that does things wrong.

Do you want to submit a PR for that?

@scottlamb
Copy link
Contributor Author

I could try, if you give me advice on how you'd like to handle it. A couple ideas:

  • special-case just a in just this one position, maybe by (in addition to updating the expected_next for the Connection::parse line_parser_once call) looping over a combination of bandwidths and attributes line_parser_once calls rather than just calling line_parser for bandwidths?
  • some more general solution involving paying less attention to order overall. Although I see order has real semantic significance in some cases like r= referring to the previous t= or obviously m= starting a Media.

@sdroege
Copy link
Owner

sdroege commented Sep 1, 2021

I would vote for the second one

scottlamb added a commit to scottlamb/sdp-types that referenced this issue Sep 7, 2021
scottlamb added a commit to scottlamb/sdp-types that referenced this issue Sep 7, 2021
@sdroege sdroege closed this as completed in 88f200f Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants