Support per-stream proxies in HLS streams (attempt 3)

[jhol]

This branch supercedes #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_proxy stream option
                      [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 configs are carried from the plugin to CFileItem via the existing CListItem::SetProperty mechanism. Thereafter, the configs are carried from CFileItem to CCurlFile and to CDVDDemuxFFmpeg via CProxy objects.

The CProxy class provided here has been simplified since #9640. All the proxy url parsing, and formatting, archiving and serialization methods have been removed, so that the CProxy is now just a simple data structure.

The class is still necessary, because without it, a large amount of boiler-plate code would be needed to pass the five proxy config values among the classes/methods in the call-flow beyond CFileItem. CProxy provides simple package in which the information can be passed around.

Furthermore, this pull request differs from #9640 in that it doesn't add a CProxy member to the CURL class. Instead it passes CProxy object side-by-side with URLs in CPlayListM3U::GetBestBandwidthStream and within the CFile family.

[FernetMenta]

I appreciate your contribution but we may find us in the 100th round if you don't start to listen. Please drop that unneeded CProxy class. It does nothing but carry those 5 properties.
The generic interface IFile won't get a method SetProxy. That generalization does not make any sense. The majority of implementations of IFile don't have a proxy.
A fileitem is passed with its properties. Components interested in the properties can parse those out.

[jhol]

@FernetMenta I'm trying to listen as best I can, but I have had some some problems interpreting your responses. I have found them somewhat terse and lacking in sufficient detail for me to correctly determine how to modify my patch-set. If you take a look at my previous pull requests you will see that CProxy only entered my patch-set in the second attempt - so this is not me repeatedly "not listening" or dismissing your comments.

I have spent quite a long time studying the code and so far this is the best patch-set I've been able to produce, though with some guidance, I'm sure it can be improved further.

So if you wouldn't mind, can you spell out the details?

1. I need a tidy method to pass proxy information through CPlayListM3U::GetBestBandwidthStream, through to the factory in CFile::Open. Given that CFile::Open has no reference to CFileItem properties - it only takes a URL string, I don't see how it could reference these values, and because of the security issue, we can no longer use the |proxy=... protocol option in the URL string itself - as I did DVDDemuxFFmpeg: Support HTTP proxies with the new http_proxy option #8680. Therefore the proxy information needs to be passed out-of-band.

2. The HLS playlist file is loaded through CFile in CPlayListM3U::GetBestBandwidthStream. Given that CFile::Open returns an anonymous IFile pointer, we can't know whether a CCurlFile will be created, so that we can call a SetProxy method on it.

   An alternative would be to attempt to cast the IFile to CCurlFile, using dynamic_cast, but this would be a very poor design in terms of separation of concerns. Another alternative would be to add a more generic key-value protocol-options system to IFile - but personally I don't see much merit in this, particularly at the cost of the type-safety that the CProxy class provides.

   Which do you want to do here?

3. How is CDVDDemuxFFmpeg meant to read the properties of CFileItem given that it only has a pointer to CDVDInputStream, and the class has no accessor method to get at the CFileItem pointer? Would you like me to add an accessor method?

So in the current state, it is necessary to copy proxy information around several "hops" through the lower levels of the HTTP request processes. Having to add multiple sets of member variables, getter methods and setter methods for each of the five variables will quickly add a large amount of ugly boiler-plate code in a half-dozen classes. Therefore, I chose to keep the CProxy class as an efficient means of passing the values around. I'm not trying to ignore you're feedback, I just didn't see how removing the class would make the code tidier.

However, if there is a way of the lower level classes getting direct access to CFileItem properties, then there would be no need to pass the config values around, but in the current code I don't see how this is possible.

[jhol]

@fritsch @basrieter - do you have comments about this?

[FernetMenta]

try to clear you mind from constraints probably set by incorrect code.

1. GetBestBandwidthStream is static and only used by CDVDInputStreamFFmpeg
   -> does not make sense and can be changed

2. All actions happen in CDVDInputStreamFFmpeg that has the object of CFileItem.

3. CDemuxStream can easily get all infos about proxy from CDVDInputStreamFFmpeg

That reduces the change from 250 loc to < 50

[jhol]

@FernetMenta

1. GetBestBandwidthStream could be moved out to CDVDInputStreamFFmpeg the reason it is presently there (and I didn't move it already) is the definitions of the m3u strings: #define M3U_START_MARKER etc. But these could be moved out to a common header.
2. I'm still not sure how you think I should get the proxy info from CFile::Open to CCurlFile?
3. Ok - I'll try that.

[FernetMenta]

2. create a CCurlFIle object instead a CFile and set the proxy info.

[jhol]

This has been superceded by #9412