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

Add VLC SAP message support #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dbussert
Copy link

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

@clux
Copy link
Owner

clux commented May 25, 2021

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.

@dbussert
Copy link
Author

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

@clux
Copy link
Owner

clux commented May 26, 2021

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.

@ibc
Copy link
Collaborator

ibc commented May 26, 2021

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

@dbussert
Copy link
Author

dbussert commented Jul 15, 2021

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]

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

Successfully merging this pull request may close these issues.

None yet

3 participants