search

DDVTECH / mistserver

The official mistserver source repository - www.mistserver.com
The Unlicense
378 stars 129 forks source link

Allow any RTP/AVP payload type to be dynamically assigned #188

Closed alexhenrie closed 5 months ago

alexhenrie commented 9 months ago

The Bosch FLEXIDOME 4000i camera sets the RTP payload type to 35 for H264, which causes MistServer to output "FAIL: Payload type 35 not supported!"

The camera probably uses 35 for H264 because the ATSC standard requires it. A/153 Part 7 states: [1]

In addition, for RTP packets that carry AVC video elementary stream, the payload_type field in the RTP header shall have value 35.

The camera also sends "a=rtpmap:35 H264/90000" to explicitly map the value 35 to H264. This is perfectly valid; although payload types 96 through 127 are recommended for dynamic use, the RTP protocol allows any AVP payload type to be dynamically reassigned. The relevant section of RFC 3551 states: [2]

This profile reserves payload type numbers in the range 96-127 exclusively for dynamic assignment. Applications SHOULD first use values in this range for dynamic payload types. Those applications which need to define more than 32 dynamic payload types MAY bind codes below 96, in which case it is RECOMMENDED that unassigned payload type numbers be used first. However, the statically assigned payload types are default bindings and MAY be dynamically bound to new encodings if needed. Redefining payload types below 96 may cause incorrect operation if an attempt is made to join a session without obtaining session description information that defines the dynamic payload types.

If the payload type is not recognized, assume that an rtpmap line will follow that defines it. If in the end there is no rtpmap line, error out in the same way whether the payload type is between 96 and 127 inclusive or whether it is outside of that range.

[1] https://prdatsc.wpenginepowered.com/wp-content/uploads/2021/04/A153-Part-7-2012.pdf#page=14 [2] https://datatracker.ietf.org/doc/html/rfc3551#page-6

Thulinma commented 9 months ago

Nicely spotted! I guess to be perfectly compatible we should actually allow overriding the detected codec based on later rtpmap lines even if the payload type is recognized... but this change is already a nice improvement over current behavior. 👍 I'll try to do a proper code review (to make sure there's no weird regressions or anything) in the coming week.

alexhenrie commented 8 months ago

@Thulinma Thanks for your feedback. Have you had a chance to look at it again? Of the three pull requests I sent, this one is the most important.

By the way, this probably goes without saying, but all of these contributions are made under the Unlicense just like the rest of MistServer is. Please let me know if you need a more formal license declaration from me or from the company I work for.

Thulinma commented 5 months ago

Merged to development branch as a1fbb9e1869320489696e83267159536281adf04. This will be part of the next release. Apologies for the long wait on this and thank you for the contribution!