Added option to disable manual injection of SPS/PPS for RTSP servers that put them in the SDP (see #3168)#3655
Open
lminiero wants to merge 1 commit into
Open
Added option to disable manual injection of SPS/PPS for RTSP servers that put them in the SDP (see #3168)#3655lminiero wants to merge 1 commit into
lminiero wants to merge 1 commit into
Conversation
…that put them in the SDP (see #3168)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #3168 we added a way for the Janus Streaming plugin to manually inject SPS/PPS NALs in the RTP stream, in case RTSP servers advertised them in the SDP via fmtp attributes. This helped address scenarios where the RTSP server would only put SPS/PPS in a
sprop-parameter-sets: WebRTC endoints never look at that, and expect them to be part of the RTP stream, so this manual injection ensured video could be decoded.This seems to not always work as expected with RTSP cameras that do advertise SPS/PPS via the SDP, but then also send SPS/PPS in the RTP stream. The end result is that, on the WebRTC side, SPS/PPS are duplicated, since there is the one sent by the camera itself, and then the one we add ourselves, which apparently confuses browsers and causes broken video.
This PR is a first attempt to fix this. It adds a new configuration property you can add when creating an RTSP mountpoint (statically or dynamically) called
rtsp_fmtp_sps: if the value of that property is"always"(default), then it behaves as it does now, and so if it finds SPS/PPS data in the SDP, it will inject it manually in RTP; if the value is"never", instead, we ignore fmtp data sent in the RTSP SDP.The property is a string and not a boolean as I want to keep doors open for more flexibility. In the future, for instance, it might be helpful to have something like an
"auto"setting, where the plugin stores the SPS/PPS found in the fmtp, but only uses it if it never sees an SPS on the wire. This would allow the code to handle more dynamically scenarios where you don't know in advance what a camera will do. That said, this automatic mode would require more code than I wanted to add now: the existing setting should already help address the main issue in scenarios where the behaviour of the cameras is known.I haven't tested this yet, so feedback is welcome. In case it works fine and with no regressions, I plan to merge soon and backport to
0.x.