RTP bridge and Chrome payload mismatch #7
Comments
|
I think the bridge needs more elaborate handling of video payload type instead of hardcoding to particular value (this will continue to break with different browser or as Chrome changes payloads supported), The code should like be a smarter version of what was done here: 40eef6d I unfortunately don't have time to to fix this myself |
After reviewing past changes and the current code in main, this honestly sounds like expected behavior. At line 104 the function RegisterDefaultCodecs is called from the upstream pion/webrtc module. This function sets up a number of codecs and PayloadTypes but importantly, 106 is not one of them as of what's in their v3.1.54 branch. Starting from commit bb7048f the custom logic that previously setup the codecs and PayloadTypes specified from CLI options was removed and replaced with RegisterDefaultCodecs from the webrtc module. This means the However it appears the CLI flag value is still used subsequently when setting up the UDP connection: see these three areas for context. It seems like the stream isn't going to be configured for PayloadType 106 (not sure what that is exactly) but that value will make it into an RTP packet at some point. I can only surmise this is causing some confusion on the receiving end of things perhaps causing the reported crash. Please feel free to critique or correct this analysis but I believe a previous commit actually led to this behavior and the behavior that occurs (not being offered the requested stream type) is in line with what the current code should do. A fix may be improved input validation on the Further, it is possible this is a bug in UE. I feel like requesting a stream type that isn't available shouldn't lead to a crash. To me that is a condition that should be handled more gracefully than a segfault. |
Oh, it certainly is.
You guys are on 4.27 based on the log, this has been fixed in subsequent versions - the crash - but requesting payload type that is not mismatched is still going to result in no streaming most likely. Based on your digging @jmpolom - it sounds like locking Pion version could also be a solution? To be clear, we don't use this code at TW, but I am happy to accept a PR if you would like to make a code change. |
This is good information. I would definitely expect if the payload type is specified more than once in an RTP packet or exchange that a mismatch would cause something to not work. Makes sense. However it sounds like there is an additional known bug in 4.27 that causes the crash which was an aspect of particular concern for @TravisBowers.
I did not review past versions/commits of the pion webrtc package. I only rather quickly glanced at the current default branch which appeared to be a release version. I did, however, examine the logic they're using to setup codec types and initially to me it seems like there would be no problem calling RegisterDefaultCodecs first and also calling RegisterCodec. It seems like the way the codecs are added is idempotent to some degree and this would allow a passed However I actually do not really have deep knowledge of the RTP protocol. I'm not sure if allowing a user to set arbitrary payload types is perhaps dangerous or simply ill advised. We could probably just as easily pin to a pion version and only allow the specific codec types that it configures as default. I do not fully understand why payload type 106 was used for the test or what codecs it corresponds with. |
|
So very high level, when doing webrtc, both sides negotiate video payload types: so in the sdp you might end up with one side saying I can do the following video payloads: h.264 - constrained baseline Each of these then gets assigned an RTP payload number to unique identify it, so like h.264 - constrained baseline might end up being 106 These numbers are not spec defined, but rather just what the browser comes up with by convention. Then the other side will read these and respond with which payload types it supports. Setting the payload type via command line was a hack from memory and proper solution would likely be to examine the full offer/answer, identify the h.264 media we want then use that as the payload. I think 106 was selected at the time because it just happened to be what Chrome and Firefox were both using - but that may have changed for a number of reasons with browser updates. edit: actually reading my own readme.md for this repo, 125 was selected as the default, so not sure how 106 came into this, but largely irrelevant, as per my explanation above. |
|
Is the assignment of payload type number specific to the client (example: chrome does it one way but safari may use different values) and primarily controlled by the client? Is there any decent set of documentation you might recommend to better understand this aspect of the rtp connection process? I did very quickly glance through RFC 3551 (rtp for audio video conferences) and RFC 6184 (h.264 payload format) and both state that the mechanism by which session participants determine dynamic payload types is beyond the scope of the RFC. I see you mentioned sdp which is something I haven't really even glanced at yet (RFC 2327?). I obviously see you have included an sdp file in this repo...is there also a network based form of sdp that is initially employed to negotiate a profile between the rtp participants? My one thought reading through the code was can we just not have to deal with manually setting payload types. I would agree with your description of it as a hack...definitely seemed like an activity that is bound to cause an issue at some point. Obviously Chrome does not require the user to manually specify a payload type so if we are to continue relying on this in our kit we should probably make real improvements along these lines. Still might be nice to keep a "manual override" for payload type present for debugging purposes but the payload should be determined dynamically.
Further comment on this...it looks like this tool already pins to v3.0.4 based on the go.mod. My comments above were in reference to a later version but it looks like they're still valid against the pinned version. |
Yeah, it is dependent on the WebRTC implementation, and ofc, FF, Chrome, Safari all have slightly different flavours. And yes, afaik, your reading of the spec is correct, the determination of payload types (numbers) is not formally defined in the spec and is instead negotiated by the peers (browsers in this case). In theory I believe payload numbers could basically be anything.
No, the thing that is employed to negotiate the profile between participants is the SDP. The SDP is the thing that is sent over the network by both peers. The general process is an "offer" SDP is sent from one peer, then that is inspected by the other side and an "answer" SDP is sent in return. The intersection of supported formats in both offer and answer is what ends up being used for the actual media.
Agreed. |
This issue seems like it may be related to #5. The RTP bridge does not appear to be using RTP video payload types passed at the command line. This will result in a type mismatch between the rtp bridge and Google Chrome pixelstreaming clients. This appears to cause the associated UE application to segfault. Even though
--RTPVideoPayloadType=106was specified upon intialization of the rtp-bridge, it looks like payload types 102, 108, 127, 125, and 123 were sent in the RTP bridge's offer and the signalling server answers with payload type 125. Later, as seen in the signalling server's log, a chrome client joins as player 102 and the signalling server appears to answer it's offer with payload type 106 (a=rtpmap:106 H264/90000). This seems to trigger a segfault in the NVENC encoder within the UE container.RTP bridge log:
Signalling server log:
Unreal Engine version: 4.27.0
UE error:
The text was updated successfully, but these errors were encountered: