webosbrew / hyperion-webos

hyperion.ng video grabber for webOS
MIT License
153 stars 30 forks source link

Add option to disable automatic HDR and powerstate switching #87

Closed sundermann closed 2 years ago

sundermann commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

This code is still missing the new arguments defined here: https://github.com/webosbrew/hyperion-webos/blob/5d33637cc5fd6bc4f1eeccdbf5d9b81b73b8b6a9/src/main.c#L77

TBSniller commented 2 years ago

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? https://github.com/webosbrew/hyperion-webos/blob/1226385d560c98bcabe61f35729e4cc333202112/src/service.c#L359

You also have to add these checks at legacy service register: https://github.com/webosbrew/hyperion-webos/blob/1226385d560c98bcabe61f35729e4cc333202112/src/service.c#L525 otherwise your options don't take affect on older webOS versions.

Thanks!

TBSniller commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Also thanks for this one!