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

RTP: remove dead-code #1953

Merged
merged 1 commit into from
Apr 25, 2023
Merged

RTP: remove dead-code #1953

merged 1 commit into from
Apr 25, 2023

Conversation

IvanNardi
Copy link
Collaborator

The checks isValidMSRTPType(..) == 1 is a subset of is_valid_rtp_payload_type() so this if-branch is never reached.

More importantly, the article describing how to detect Microsoft Lync and Skype for Business is from 2014. These payload types are static or they are in the dynamic range: in both cases, these values might be used (and they are used indeed) pretty much by every application. Bottom line: we can't use PT alone to identify a specific protocol.

Keep the list, since it is used to tell audio streams from video ones.

The checks `isValidMSRTPType(..) == 1` is a subset of
`is_valid_rtp_payload_type()` so this if-branch is never reached.

More importantly, the article describing how to detect Microsoft Lync and
Skype for Business is from 2014. These payload types are static or they
are in the dynamic range: in both cases, these values might be used (and
they are used indeed) pretty much by every application.
Bottom line: we can't use PT alone to identify a specific protocol.

Keep the list, since it is used to tell audio streams from video ones.
@sonarcloud
Copy link

sonarcloud bot commented Apr 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@IvanNardi IvanNardi merged commit de693cb into ntop:dev Apr 25, 2023
@IvanNardi IvanNardi deleted the rtp-dead branch April 25, 2023 12:16
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.

2 participants