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
Provide media headers to inputstream.adaptive if channel does not explicitly set them. #352
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think you are doing more than you need to with this. It looks like you strip the headers to just add them in the same way again. Can you give some examples of URLs and what the value of stream_headers
ends up being?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really you should update the AddHeader call to optionally support URL encoding. I think your code to encode the headers could still be simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think you should add a boolean to
std::string StreamUtils::AddHeaderToStreamUrl(const std::string& streamUrl, const std::string& headerName, const std::string& headerValue) |
And then do the URL encoding as part pr that function. You could change the name to something else if you wish. Like AddHeader
and the argument just called a simple string name or maybe headerTarget
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this is looking really good. Just some minors.
You also need to update the addon.xml.in and changelog.txt. There are chagnelog entries in both files.
For the version number you need to update minor number. I.e. in major.minor.patch the middle one as you've added a new feature.
Well done ;)
if channel does not explicitly set them. Fixes the HTTP 403 error on segment download with several IPTV providers, as suggested by Markus Pfau (@peak3d) here: xbmc/inputstream.adaptive#411 Signed-off-by: Vasyl Gello <vasek.gello@gmail.com>
Technically it is a bugfix so I changed the patch instead of minor :) With timeshift occupying 4.12.0 in the PRs list, would be good to make it live ;) Thanks for review! :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
And thanks for contributing! |
Fixes the HTTP 403 error on segment download with several
IPTV providers, as suggested by Markus Pfau (@peak3d)
here: xbmc/inputstream.adaptive#411