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

Add option to disable automatic HDR and powerstate switching #87

Merged
merged 2 commits into from
Oct 2, 2022

Conversation

sundermann
Copy link
Contributor

On my TV globally enabled HDR gives the best results even on SDR content. I assume that is because SDR content is converted to HDR internally resulting in washed out colors if HDR tone mapping is not enabled. This PR adds an quirk which can be set to force HDR state on all content.

@TBSniller
Copy link
Collaborator

Hey there,
first thanks for your contribution!
Maybe I don't get it, but where is the part in which it is forced to be enabled? I can only see disable of callback functions, but not the forcefully enable part before.
I've also seen a parameter to disable powerstate check. Do you mind shortly documenting this too?
We have also merged unicapture branch into main. Would be nice if you can change your PR to merge into main too.

Thanks!

@sundermann sundermann changed the base branch from unicapture to main September 27, 2022 20:52
@sundermann sundermann changed the title Add quirk to force HDR state to enabled Add option to disable automatic HDR and powerstate switching Sep 27, 2022
@sundermann
Copy link
Contributor Author

The title is a bit misleading as this PR was updated following the suggestions by @tuxuser. This is no longer for forcing the HDR state but rather adding the option to disable automatic HDR and screensaver switching.

Regarding forcing the HDR state, I went another route in the meantime by adding tonemapping in hyperion-webos rather than relying on HyperHDR.

@tuxuser
Copy link
Member

tuxuser commented Sep 28, 2022

This code is still missing the new arguments defined here:

while ((opt = getopt_long(argc, argv, "x:y:a:p:f:b:u:q:c:vnhdVG", long_options, &longindex)) != -1) {

@TBSniller
Copy link
Collaborator

TBSniller commented Oct 2, 2022

Hey,
just tested this out. When running as service and setting these settings at runtime, they will not take any affect until next full service restart.
Maybe you can add another check in callback-functions and return if it shall be disabled?

static bool videooutput_callback(LSHandle* sh __attribute__((unused)), LSMessage* msg, void* data)

You also have to add these checks at legacy service register:

if (registeredLegacy) {
otherwise your options don't take affect on older webOS versions.

Thanks!

@TBSniller
Copy link
Collaborator

And can you please lint your changes with clang?

@@ -504,[20](https://github.com/TBSniller/hyperion-webos/actions/runs/3170003190/jobs/5162296017#step:11:21) +504,20 @@
 
     if (!service->settings->no_powerstate) {
         if (!LSCall(handle, "luna://com.webos.service.tvpower/power/getPowerState", "{\"subscribe\":true}",
-                    power_callback, (void *) service, NULL, &lserror)) {
+                power_callback, (void*)service, NULL, &lserror)) {
             WARN("Power state monitoring call failed: %s", lserror.message);
         }
     }
 
     if (!service->settings->no_hdr) {
         if (!LSCall(handle, "luna://com.webos.service.videooutput/getStatus", "{\"subscribe\":true}",
-                    videooutput_callback, (void *) service, NULL, &lserror)) {
+                videooutput_callback, (void*)service, NULL, &lserror)) {
             WARN("videooutput/getStatus call failed: %s", lserror.message);
         }
 
         if (!LSCall(handle, "luna://com.webos.settingsservice/getSystemSettings",
-                    "{\"category\":\"picture\",\"subscribe\":true}", picture_callback,
-                    (void *) service, NULL,&lserror)) {
+                "{\"category\":\"picture\",\"subscribe\":true}", picture_callback,
+                (void*)service, NULL, &lserror)) {
             WARN("settingsservice/getSystemSettings call failed: %s", lserror.message);
         }
     }

We have added linting in latest unicapture branch PR before merging to main: https://github.com/webosbrew/hyperion-webos#code-style--linting

@sundermann sundermann force-pushed the force-hdr-quirk branch 2 times, most recently from 68a679f to f1cd996 Compare October 2, 2022 21:43
@sundermann
Copy link
Contributor Author

I have addressed your comments. Also the opposite was true: When HDR switching was off and a user enabled it, the callbacks would not have been registered. To fix this, I have completely removed the conditional checks on the service registration calls and instead moved to checking the current state inside the callbacks as you suggested.

@TBSniller
Copy link
Collaborator

Also thanks for this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants