sdp-transform icon indicating copy to clipboard operation
sdp-transform copied to clipboard

Add VLC SAP message support

Open dbussert opened this issue 3 years ago • 5 comments

SAP messages for MPEG-TS UDP video have a /x to the port like 8208/1. Wireshark calls this Media Port Count. The media filter of \d does not match this, so it's not parsed. This pull request adds a grammar regex that matches a line like m=video 8208/1 udp 33

image

dbussert avatar May 25 '21 14:05 dbussert

Ah, thanks for this.

I think it's probably better to widen the existing regex to capture it if that's possible. AFAIKT it's only the \d that's causing the problem, and everything else is identical. I'd rather not have more than one m line regex because m-line semantics is kind of wild.

A quick test sdp for this would be very helpful.

clux avatar May 25 '21 16:05 clux

do you want the regex to be \d and only the char / to be minimalist?

dbussert avatar May 26 '21 01:05 dbussert

i think you can still have something that is effectively (\d*)/(*/d), but that only gets the port count parameter if there's a slash

possibly (\d*)(?:\/(\d*)).. the rfc4566 indicates that this is all within-spec, and we should have had support in the first place.

clux avatar May 26 '21 07:05 clux

Please don't add a new grammar rule just to parse an optional field that was missing in the existing grammar rule.

ibc avatar May 26 '21 10:05 ibc

finally got around to integrating those 2 rules together. I ended up with /^(\w*) (\d*)(?:\/(\d*))? ([\w/]*)(?: (.*))?/ which adds an optional match for a port number while excluding the / divider with a non-matching group

No port count makes portCount undefined

'video 8208 udp 33'.match(/^(\w*) (\d*)(?:\/(\d*))? ([\w/]*)(?: (.*))?/)
(6) ["video 8208 udp 33", "video", "8208", undefined, "udp", "33", index: 0, input: "video 8208 udp 33", groups: undefined]

A / and port count after the port is now captured

'video 8208/1 udp 33'.match(/^(\w*) (\d*)(?:\/(\d*))? ([\w/]*)(?: (.*))?/)
(6) ["video 8208/1 udp 33", "video", "8208", "1", "udp", "33", index: 0, input: "video 8208/1 udp 33", groups: undefined]

The one issue is with \d* it will capture an empty string if there is a / but no port count defined

'video 8208/ udp 33'.match(/^(\w*) (\d*)(?:\/(\d*))? ([\w/]*)(?: (.*))?/)
(6) ["video 8208/ udp 33", "video", "8208", "", "udp", "33", index: 0, input: "video 8208/ udp 33", groups: undefined]

dbussert avatar Jul 15 '21 21:07 dbussert