Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

embedding streamid in url leads to usage issue #1871

Closed
quink-black opened this issue Mar 16, 2021 · 16 comments
Closed

embedding streamid in url leads to usage issue #1871

quink-black opened this issue Mar 16, 2021 · 16 comments
Labels
[apps] Area: Test applications related improvements Type: Bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@quink-black
Copy link
Contributor

quink-black commented Mar 16, 2021

Embedding streamid in url is a common usage. AccessControl recommend streamid value starts with '#', for example:

#!::u=admin,r=foo

This leads to url like

srt://example.com:9000?streamid=#!::u=admin,r=foo

Now, "streamid=" is a query, "#!::u=admin,r=foo" is not the query's value, but a fragment.
https://tools.ietf.org/html/rfc3986#section-3.4

This leads to a usage issue: we can't use some standard library to parse the url, unless treat the fragment specifically as streamid value.

@quink-black quink-black added the Type: Bug Indicates an unexpected problem or unintended behavior label Mar 16, 2021
@ethouris
Copy link
Collaborator

ethouris commented Mar 17, 2021

Well, this example indeed isn't very fortunate, but this is the URI that can be parsed by our example applications. This means nothing for the SRT library because it doesn't do URI parsing at all. This should be understood as setting - in this case - the SRTO_STREAMID socket option to a string "#!::u=admin,r=foo".

The standard URI should of course sound as srt://example.com:9000?streamid=#&excl::u=admin,r=foo.

Still, this document describes only the socket option and doesn't mention the URI you showed, so I don't understand the problem.

@maxsharabayko
Copy link
Collaborator

Looks like some quotation marks ('query') could be used to define the stream ID value as the URI query. 🤔

srt://example.com:9000?streamid='#!::u=admin,r=foo'

@maxsharabayko maxsharabayko added this to the v1.4.4 milestone Mar 17, 2021
@maxsharabayko maxsharabayko added the [apps] Area: Test applications related improvements label Mar 17, 2021
@ethouris
Copy link
Collaborator

Not sure if this will work, but again, this is a shell trick. This kind of URI as a whole isn't used anywhere in our documentation.

grep '?streamid=' -r *

in the docs directory returns nothing. Phrase streamid= is only found in the documentation for the srt-test-multiplex demo/test application and not as URI.

@quink-black
Copy link
Contributor Author

Let me summarize the question: Is it a valid use case to embedding streamid in url?

  1. If it is, the documentation should cover the use case, including a best practice, to make good compatibility between different client/server.

  2. If it's not a good use case, disapprove the behavior in the documentation explicitly.

@ethouris
Copy link
Collaborator

It is a valid case to set a socket option of string type to a string value containing printable characters. For streamid case, it is a valid case to call srt_setsockflag (or srt_setsockopt) function using SRTO_STREAMID and the string specification as a value, as well as extract this option's value from the socket.

All other things are out of the scope of the SRT library. How an application receives parameters for itself that should be transformed into the SRT socket options is within the sole scope of the application. The URI used in the demo and testing applications serve only as a bridge between the library API and the shell command. It should not serve as a "URI model" for applications using SRT. Maybe this should be written somewhere explicitly in the documentation.

@ethouris
Copy link
Collaborator

Just to add something as I've checked how things look like in the documentation: The URI specification is used in the documentation for srt-live-transmit application, with some mentions also in socket-groups.md and the srt-test-multiplex application. The GStreamer application is using a different approach: ! srtserversink uri=srt://0.0.0.0:8888/ latency=100. I think only for the documentation of srt-live-transmit application there should be a mention about what this URI is.

@maxsharabayko
Copy link
Collaborator

@quink-black
Sorry, I was not clear in my previous comment.

We don't have a dedicated SRT URI documentation yet, however, supported URI querys can be found in srt-live-transmit.md. streamid is the supported URI query.

Passing StreamID in URI is one of the use cases. Most products though use alternative methods to configure SRT, e.g. UI, REST API, etc. Still, handling the streamid in URI is something to define, maybe even in the Access Control Guidelines.

My comment regarding handling the quotation marks ('query') was about adding this kind of handling in srt-live-transmit.
URI srt://example.com:9000?streamid=#!::u=admin,r=foo is not a URI where #!::u=admin,r=foo can be treated as a value for the streamid query. So we need to mask it somehow, e.g. with the quotation mark character '`'.
It needs to be checked as well if the terminal will handle this type of syntax correctly.

Might be there are other possibilities to resolve this. Feel free to suggest.

@quink-black
Copy link
Contributor Author

quink-black commented Mar 17, 2021

I know VLC and FFmpeg both support the use case and support setting streamid by option. Since FFmpeg av_find_info_tag does little URL decoding, and the srt plugins in VLC don't use vlc_UrlParse but write from scratch, they should work with the format specified by "Access Control Guidelines". Actually I was trying to clean up some code in VLC and found out streamid doesn't work after refactor.

Some suggestions:

  1. Remove the prefix '#' when compose url, and add it back before pass to srt_setsockopt(). However, it doesn't work if the server doesn't use the format specified by "Access Control Guidelines" in which case client shouldn't add a prefix.
  2. Change streamid format to use different prefix. Let server handle different formats for backward compatible.

@ethouris
Copy link
Collaborator

ethouris commented Mar 17, 2021

The only way how applications can accept URI so that it is standard is:

  • Accept URIs with standard encoding - in this case, e.g. srt://example.com:9000?streamid=%23%21%3A%3Au=admin,r=foo [EDIT: fixed]
  • Make appropriate URI parsing and entity decoding so that this result in a string to be passed to socket options - that is, it should result in setting "#!::u=admin,r=foo" to SRTO_STREAMID socket option.

@quink-black
Copy link
Contributor Author

The only way how applications can accept URI so that it is standard is:

  • Accept URIs with standard encoding - in this case, e.g. srt://example.com:9000?streamid=#!::u=admin,r=foo

I think URL encoding use Percent-Encoding, not html encode.

@ethouris
Copy link
Collaborator

Right, of course. This should be srt://example.com:9000?streamid=%23%21%3A%3Au=admin,r=foo.

@quink-black
Copy link
Contributor Author

If url encoding is the way to go, the next question is: should client do url decoding and pass the decoded streamid to libsrt, or should client pass the encoded streamid to libsrt and let server handle the decoding process? The url can come from a signal server, so let client do passthrough can improve interoperability.

@ethouris
Copy link
Collaborator

I think this answers this question well enough.

  • Accept URIs with standard encoding - in this case, e.g. srt://example.com:9000?streamid=%23%21%3A%3Au=admin,r=foo [EDIT: fixed]
  • Make appropriate URI parsing and entity decoding so that this result in a string to be passed to socket options - that is, it should result in setting "#!::u=admin,r=foo" to SRTO_STREAMID socket option.

As for using this URL by applications, transforming, sending, etc. - this is the application layer and out of scope for the library. The library only needs that the SRTO_STREAMID socket option be appropriately set and the AccessControl.md document doesn't speak about anything else.

@quink-black
Copy link
Contributor Author

Let client do url decoding is only one of many choices. In my opinion, this project is more than a library, it should consider what's the best practice for interoperability, how the downstream software works with libsrt.

@wakabayashik
Copy link

I think this issue is what I posted before; #1173
('!' and ':' can be included directly in the query part without pct-encoding)

@maxsharabayko
Copy link
Collaborator

Resolved by #2015.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[apps] Area: Test applications related improvements Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

4 participants