xbmc icon indicating copy to clipboard operation
xbmc copied to clipboard

[addons][inputstream] Add new SetVideoResolution with max resolution

Open CastagnaIT opened this issue 3 years ago • 4 comments

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

CastagnaIT avatar Apr 27 '22 14:04 CastagnaIT

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

da-anda avatar May 07 '22 11:05 da-anda

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 rate setting is enabled/disabled (that this kodi setting should be renamed to Adjust resolution / framerate to 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

CastagnaIT avatar May 07 '22 12:05 CastagnaIT

jenkins build this please

CastagnaIT avatar May 24 '22 09:05 CastagnaIT

@AlwinEsch requested changes has been done can you finish the review?

CastagnaIT avatar Aug 05 '22 06:08 CastagnaIT

Looks good!

AlwinEsch avatar Aug 13 '22 09:08 AlwinEsch

@CastagnaIT I will do the rebase right now, then merge.

ksooo avatar Sep 16 '22 06:09 ksooo

thank you

CastagnaIT avatar Sep 16 '22 06:09 CastagnaIT