[addons][inputstream] Add new SetVideoResolution with max resolution
Description
Currently from SetVideoResolution callback we provide current GUI resolution,
but if Adjust refresh rate feature is enabled the GUI resolution is no longer relevant
Adjust refresh rate will change screen resolution based on stream resolution (takin in account also whitelist, if any) OFC this happens when the video stream in the demuxer
therefore for InputStream addons, Adjust refresh rate cannot works correctly,
because the addon has as reference the GUI resolution then we will send in demuxer a stream resolution with GUI resolution instead the max allowed resolution of display (switchable by Adjust refresh rate)
so having the max resolution info before the playback take place, allow addon to provide in the demuxer the best stream resolution that can fit the supported screen resolution (changed when playback will start).
~Is kept compatibility to original SetVideoResolution, then no changes are required to existings inputStream addons~
Plus feature: provide callback also when the resolution is changed while in playback, e.g. for window resize allow us to change stream quality to fit current screen resolution and then optimize bandwidth (adaptive stream technology)
Motivation and context
Alternative to PR #21296
Often users use Adjust refresh rate feature and whitelist to make match stream resolution to the screen resolution at playback start
but this feature prevent us to provide the right stream resolution at playback start, and make adaptive stream technology to work partially because we know only the GUI resolution provided before the playback
How has this been tested?
see log when callback happens
void CInputStreamAdaptive::SetVideoResolution(unsigned int width, unsigned int height, unsigned int maxWidth, unsigned int maxHeight)
{
LOG::Log(LOGINFO, "SetVideoResolution (%ux%u max: %ux%u)", width, height, maxWidth, maxHeight);
}
What is the effect on users?
An example is allow to auto-switch kodi resolution at 4k having GUI at 1080P In future with InputStream Adaptive
Screenshots (if appropriate):
Types of change
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Clean up (non-breaking change which removes non-working, unmaintained functionality)
- [ ] Improvement (non-breaking change which improves existing functionality)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that will cause existing functionality to change)
- [ ] Cosmetic change (non-breaking change that doesn't touch code)
- [ ] None of the above (please explain below)
Checklist:
- [x] My code follows the Code Guidelines of this project
- [ ] My change requires a change to the documentation, either Doxygen or wiki
- [ ] I have updated the documentation accordingly
- [x] I have read the Contributing document
- [ ] I have added tests to cover my change
- [ ] All new and existing tests passed
sorry, might be a stupid question since I'm not that deep into all this, but doesn't the resolution whitelist also depend on the fps of the video? I don't see your code taking the fps into account
most of the time the framerate are not importants to "adaptive stream" stream selection, video services usually do not provide video adaptation set's with multiple framerate, but: there may be some cases maybe less common (youtube is the only that i know) that can provide adaptation sets with differents framerate types
this is one of the reasons why i had initially made a PR to provide the full list of resolution information would help us select the best stream frame rate, a case to study better later there are different things to do on ISAdaptive...
i think the best would be:
- have full list of resolutions (the old PR liked in description)
- a way to know when
Adjust refresh ratesetting is enabled/disabled (that this kodi setting should be renamed toAdjust resolution / framerateto prevent misunderstandings) - a callback to know when the resolution is changed while in playback (eg window resized)
will cover all use cases but if is not allowed, the only alternative solution that i see is something like this PR
jenkins build this please
@AlwinEsch requested changes has been done can you finish the review?
Looks good!
@CastagnaIT I will do the rebase right now, then merge.
thank you