search

KallDrexx / rust-media-libs

Rust based libraries for misc media functionality
Apache License 2.0
229 stars 58 forks source link

Add optional tcUrl field for request_connection #18

Closed JonathanUsername closed 3 years ago

JonathanUsername commented 3 years ago

Hi, thanks for this library, it's extremely useful!

I've been trying to use it as a basis for a sort of proxy for incoming RTMP connections and I ran into a surprising issue. Our push target RTMP server is SRS (https://github.com/ossrs/srs) and using the example mio_rtmp_server to test against, it gave this response:

serve error code=2005 : rtmp connect tcUrl : invalid request without tcUrl

I assume this isn't a hard requirement as part of the RTMP specification that your library follows, but I have found references to it here https://linux.die.net/man/8/rtmpgw and here https://en.wikipedia.org/wiki/Real-Time_Messaging_Protocol#Protocol, which suggests it's diffuse.

I initially "vendored in" this library by copying it and making the changes I needed within my own project, but it would be great if your library could offer this additional feature as it seems I can't use it without it. I couldn't find a nice way to work around Rust's visibility rules to make the changes unobtrusively from my code. I afraid I'm fairly new to Rust so it could be there's a nicer way that I can't see, but since most of rtmp::sessions::client is private, I thought it was unlikely I could monkey-patch anything or call lower-level functions to make a bespoke Amf0Command with the added field.

So this is my simple proposal for adding this functionality. I kept it as optional so that it wouldn't break backwards compatibility. If it's not idiomatic Rust please let me know, I'd like to use this as an opportunity to learn.

KallDrexx commented 3 years ago

Uploaded change with 0.3.8