search

tellytv / telly

An IPTV proxy
MIT License
769 stars 107 forks source link

More robust URI parsing in M3U files #286

Closed mulbc closed 3 years ago

mulbc commented 3 years ago

Adds:

This should Resolve #277

chazlarson commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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?