Ampersand as rtmp path separator prevents sending of query parameters

[JacobRichardt]

I recently had to add a video player for a third party streaming service. The streams came from a Wowza server with a custom plugin that requires query parameters in the server path, like so:

Server: rtmp://stream.server.com:1938/path?parameter1=value1&parameter2=value2
Stream: flv:file.flv

Since the server part contains ampersands, videojs interprets the path incorrectly when combining the two parts with an ampersand. Videojs also supports another scheme as a fallback, where instead of the first ampersand the last slash is used. But as far as I can see it is only provided as a fallback. However forcing that scheme by altering the videojs source solved the problem for me.

I'm guessing the first scheme is used to solve another problem (slashes in the stream part?). Otherwise I would suggest simply removing that scheme.

If that is not possible, maybe the character used could be changed to something that does not has special meaning in an url. Or maybe videosjs could decode url encoded ampersands after separating the two parts? Or just an option to disable the ampersand scheme.

[bdeitte]

I think if the existing scheme was changed to ignore everything after "?", that would work for you as well as the existing people using this feature. If you wanted to add a PR for this, that seems reasonable. I don't think anybody in the usual video.js contributors list is going to pick this up, as they are focused on the non-Flash streaming solutions.

You are right that the "&" was added to solve slashes in the stream. This has been the solution used at Brightcove for a long time, and I brought in the changes here to try to support existing URLs.

[JacobRichardt]

I'd like to contribute, but skimming through the contrib guidelines this seems a bit too time consuming considering it is a one line change.
If someone already involved in the project wishes to implement the fix, I've forked and made the change here: JacobRichardt@fc048a0

[gkatsev]

@Lillemanden submit that as a PR. There really isn't much more to it.

[JacobRichardt]

Aright, maybe I just looked like there was a lot more too it. PR has been submitted.

[gkatsev]

The other stuff in there describes how to clone the repo and get it running locally so you can test it before submitting a change. While it is preferable if someone submitting a PR does this it's possible to get away without doing it because we have tests running on a CI.

Also, generally we leave the issues open up until the associated PR gets merged.

[santigracia]

same problem here with a url from livecoding.tv
livecoding.tv is using a similar format, like Wowza, but only 1 parameter.
rtmp://usmedia2.livecoding.tv:1935/livecoding:santi?t=E0F7D3F305444673B646B6645ABE44C9

[gkatsev]

This has been merged and released in 5.2, you can grab 5.2.1 on npm or github releases.