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

More robust URI parsing in M3U files #286

Merged
merged 4 commits into from
Nov 2, 2021
Merged

More robust URI parsing in M3U files #286

merged 4 commits into from
Nov 2, 2021

Conversation

mulbc
Copy link
Contributor

@mulbc mulbc commented Oct 17, 2021

Adds:

  • debug output for how we execute ffmpeg
  • Better URI parsing in M3u files to support rtsp URIs (for FritzBox boxes)

This should Resolve #277

@chazlarson
Copy link
Contributor

Does this work if ffmpeg is disabled? In that case it's just going to pass the URL to Plex, and I don't think Plex handles rtsp URLs.

@mulbc
Copy link
Contributor Author

mulbc commented Oct 17, 2021

Indeed, rtsp links won't work without ffmpeg, but with ffmpeg this would work. In my opinion though, the parser shouldn't limit URIs to HTTP and UDP links, but accept whatever URI is in there. Even with HTTP and UDP links we don't know if that would work without ffmpeg (the reason why we have the option I assume)

The main reason for me to use Telly is to Proxy the LiveTV that is offered by my FritzBox to Plex. Plex has a builtin beta for doing this directly, but it's rather instable. Going via Telly appears to be a bit more stable.
But I need the change in this PR in order to use my FritzBox for this... Because the FritzBox only offers rtsp links

@chazlarson
Copy link
Contributor

The right answer then is to broaden the acceptance of URL schemes to accept those that work with the current config, not open the door to any URLs at all without regard for whether the target environment accepts them.

@mulbc
Copy link
Contributor Author

mulbc commented Oct 18, 2021

What I tried to say is that the protocol isn't too important in the decision if Plex supports it right now (or in the future). Even without ffmpeg and directing Plex directly to the rtsp link, it doesn't fail immediately. Therefore I assume, Plex generally supports rtsp, but not this particular video format or container.

Therefore even without this PR we could have HTTP or UDP links that Plex wouldn't be able to handle without ffmpeg.

So my reasoning:
With this PR, people with a FritzBox will be able to use this if they enable ffmpeg. Without this PR, there is no way for them to use their FritzBox.

One thing that I could offer: If people have URIs that are not http or udp AND don't have ffmpeg enabled, we could print a warning to the log stating that ffmpeg will most likely be needed.

@chazlarson
Copy link
Contributor

Fair point, but my next question would be why not add rtsp rather than just accepting whatever? Of course, letting telly accept rtsp links adds functionality in this case and enables use of this device, but why is the solution to accept any URL at all? I typically prefer enabling just what needs to be enabled, rather than throwing errors after the fact about things that perhaps should have been blocked before attempting.

Rather like websites that let you enter a new password, only to tell you afterwards that it doesn't meet some requirement.

@mulbc
Copy link
Contributor Author

mulbc commented Oct 19, 2021

I didn't see a reason why we should limit it to the protocols we anticipate right now, when there are many more possible protocols out there. Especially with the mighty ffmpeg we can probably support a fair amount of protocols.

If you really only want to have http, udp and rtsp protocols, I can do that, but then I'd rather not have that logic in the m3u parser, but later in the actual main code.

Let me know what you prefer.

@chazlarson
Copy link
Contributor

chazlarson commented Oct 25, 2021

For me it's a not a matter of whether or not ffmpeg potentially supports different protocols. It's more that the two that are currently allowed are typically supported with or without ffmpeg. Sure, there may be some specific http or udp links that don't work with Plex directly, but in the case of rtsp, it requires ffmpeg, as would many in the list of possible protocols you mention. In my opinion, if telly is going to allow a protocol that requires ffmpeg be turned on, it should enforce that requirement.

Personally I would rather the behavior be "if the m3u contains a protocol that requires ffmpeg be enabled but ffmpeg is not enabled, exit with a descriptive error", similar to how it behaves with excessive numbers of channels. Allowing it through only for the Plex black box to fail with a vague but totally predictable error later is bound to result in user confusion as to why it's failing.

That's also why I'd prefer to limit the list to known good protocols that are actually seen in the the wild. If everything gets let through, now you've got users wondering why this file:// or whatever link isn't working when there's no expectation it even should.

mulbc added 2 commits November 2, 2021 11:22
That function nowadays doesn't ever return an error,
but that might change in the future
Also moved URI of Track to net/url for better URI handling
@mulbc
Copy link
Contributor Author

mulbc commented Nov 2, 2021

I implemented a check in lineup.go to check if ffmpeg is enabled for URIs that are neither HTTP or UDP (it will issue a warning otherwise)
I think this is what you wanted, right?

@chazlarson chazlarson merged commit 6208fd1 into tellytv:dev Nov 2, 2021
@mulbc mulbc deleted the dev branch December 6, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants