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

Support per-stream proxies in HLS streams (attempt 5) #9421

Merged
merged 8 commits into from Mar 22, 2016

Conversation

jhol
Copy link
Contributor

@jhol jhol commented Mar 22, 2016

This branch supercedes #9412, #9398, #9240 and #8680, providing another attempt to add the ability for plugins to provide per-stream proxy configuration, including support for proxies with HLS.

The flow of URL information from plugins to HTTP requests for HLS streams is as follows:

[Plugin]
  -> CListItem
      -> CFileItem
          -> CDVDInputStream/CDVDInputStreamFFmpeg
              -> CPlayListM3U::GetBestBandwidthStream
                  -> CFile::Open
                      -> IFile/CCurlFile
                          -> libcurl
                              [HTTP Request playlist M3U8 file]

          -> CDVDInputStream
              -> CDVDDemuxFFmpeg
                  -> libffmpeg
                      [HTTP Request media chunks]

To enable per-stream proxy configuration, the five config values (proxy protocol, host, port, user-name and password) must follow the same path-ways.

  • The proxy configurations are carried by five new CFileItem properties: proxy.type, proxy.host, proxy.port, proxy.user and proxy.password.
  • Accessor methods (GetProxyType etc.) have been added to CDVDInputStreamFFmpeg, which expose these property values.
  • CPlayListM3U::GetBestBandwidthStream has been moved to CDVDInputStreamFFmpeg, so that it can gain access to the proxy information from CDVDInputStreamFFmpeg.
  • Use of the CFile::Open factory mechanism has been removed from this method. Instead CCurlFile is created directly, so that the proxy values can be applied to the object.
  • CDVDDemuxFFmpeg has access to CDVDInputStreamFFmpeg, and adds the http_proxy string to the options dictionary if one has been specified.

@garbear
Copy link
Member

garbear commented Mar 22, 2016

Please force-push instead of opening new pull requests. When you rewrite history and force-push to a branch of the same name, github will automatically update the PR. Note that any comments directly attached to old commits will be hidden (but still viewable)

if (selected.compare(m_item.GetPath()) != 0)
const CURL playlist_url = m_item.GetURL();
const CURL selected = PLAYLIST::CPlayListM3U::GetBestBandwidthStream(playlist_url, bandwidth);
if (selected.Get().compare(playlist_url.Get()) != 0)

This comment was marked as spam.

@garbear
Copy link
Member

garbear commented Mar 22, 2016

I went through the code and it looks like you're on the right track. Good job.

{
const std::string type_str = GetProxyType();
CCurlFile::ProxyType type = CCurlFile::PROXY_HTTP;
if (type_str == "http")

This comment was marked as spam.

@FernetMenta
Copy link
Contributor

I agree with @garbear and only have this single comment. Then it is good to go 👍

@FernetMenta FernetMenta added Type: Improvement non-breaking change which improves existing functionality v17 Krypton Component: Video labels Mar 22, 2016
@FernetMenta FernetMenta added this to the Krypton 17.0-alpha1 milestone Mar 22, 2016
Formatting the CURL as a string, then parsing it back to a CURL
is redundant, and has the effect of stripping any additional CURL
object metadata.
Since v3.0, FFmpeg has supported the http_proxy option for the HTTP
protocol.

Previously, CDVDDemuxFFmpeg would pass Kodi httpproxy CURL protocol
option through as a HTTP header, which the server would (hopefully)
ignore. With this change, if Kodi is built with an older version of
FFmpeg, FFmpeg will simply ignore the option - which is an
improvement over sending spurious HTTP headers.
@jhol
Copy link
Contributor Author

jhol commented Mar 22, 2016

@garbear @FernetMenta - now updated

@FernetMenta
Copy link
Contributor

@jhol thank you very much!
jenkins build this please

@FernetMenta
Copy link
Contributor

@jhol congrats and thanks again

FernetMenta added a commit that referenced this pull request Mar 22, 2016
Support per-stream proxies in HLS streams (attempt 5)
@FernetMenta FernetMenta merged commit 61d6149 into xbmc:master Mar 22, 2016

// Select the standard port
const std::string value = GetProxyType();
if (value == "socks4" && value == "socks4a" &&

This comment was marked as spam.

@kyl416
Copy link

kyl416 commented Jul 29, 2020

Is there any documentation or examples on how to use these proxy properties in a strm file? I tried "CFileItem.proxy.host" and it didn't work.

EDIT: I got it working with "proxy.host", the problem though for m3u8 it seems to only affect the playlist while http URLs that redirect to https don't use the proxy. And I couldn't get it to work with socks5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Improvement non-breaking change which improves existing functionality v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants