univrsal / input-overlay

Show keyboard, gamepad and mouse input on stream
GNU General Public License v2.0
2.91k stars 246 forks source link

Fix hdr display #334

Closed anchaides closed 1 year ago

anchaides commented 1 year ago

updated overlay to initialize image of type gs_image_file4, there's seems to be some back end shenanigans in OBS where calling gs_image_file4_init_texture(m_image); result in the texture being generated using a color space corresponding to the scene.

Also updated element_texture::draw to enable blending mode, otherwise HDR color spaces would overwrite overlapping pixels even though the overlapping pixels were transparent to begin with. ( as is the case with layouts as the provided wasd) so it looks like a shadow is cast from the newer elements.

e.g.,

image

gs_image_file4_init requires a linear alpha value to be defined. Which I also added to initialization.

anchaides commented 1 year ago

I also have a local commit where I tied the linear alpha setting to a checkbox, similar to how "image_source" does it. I haven't pushed it yet because I tied its change to the same "file_changed" function as the image source and image layout properties.

`bool file_changed(void d, obs_properties_t props, obs_property_t , obs_data_t const auto config = obs_data_get_string(data, S_LAYOUT_FILE);

 auto old_image_file = src->m_settings.image_file;

 src->m_settings.image_file = obs_data_get_string(data, S_OVERLAY_FILE);

auto old_linear_alpha  = src->m_settings.linear_alpha;

src->m_settings.linear_alpha = obs_data_get_bool(data, S_LINEAR_ALPHA);

 /* Only reload config file if path changed */
if (src->m_settings.layout_file != config || src->m_settings.image_file != old_image_file ||
src->m_settings.linear_alpha != old_linear_alpha) {
     src->m_settings.layout_file = config;

     if (!src->m_overlay->load()) {
         src->m_settings.layout_flags = 0;`

However, this seems a little wasteful since the linear alpha value should only affect texture initialization, i.e., gs_image_file4_init_texture(m_image);

In this commit I have added the appropriate linear alpha text strings imported from all the matching locale files from input_source

Should I push it as is?

anchaides commented 1 year ago

Another option is to have a call back function in input_source.cpp updating linear_alpha property value and have that calling overlay::load_texture only as in the last commit. But that requires making overlay::load_texture pubic

Let me know what you think

univrsal commented 1 year ago

I gave this a quick test and it seems to work fine for me, I have some questions though:

I also ran clang-format and (hopefully) fixed a memory leak and created a pull request for those changes on your fork.

anchaides commented 1 year ago
  • Do I need to change any of the color settings in obs to properly test this?

Yes, without the PR. Changing OBS settings in "Advanced -> Video: Color Format -> P010 (10-bit, 4:2:20, 2 planes) Color space -> Rec 2100 (PQ) Color Range -> Full Will have you loading wasd provided overlay like this

image

  • What am I supposed to see to know that it's working (can I even tell without having an HDR capable screen?)

Even if you don't have an HDR capable display OBS's renderer will mess up the image in the scene preview. If it works, overlay will be displayed normally. As i Understand it this color space contains the non HDR one so recording in this format without an HDR capable display from a non HDR video source would generate a normal video with a larger file size.

  • From my initial testing it felt to me like "apply alpha in linear space" should be on by default (maybe just for non-hdr recordings?)

Obs included the linear alpha in gs_image_file4 type as part of its features, which is required for initialization. Which is why I added it to my fork, since image-source also provides this setting. While I'm not entirely sure about this, it seems that since gs_image_file4 is used to import a PNG image and display it on a video format by generating a texture that's rendered and drawn using the Open GL libraries used by OBS, the renderer might do some gamma corrections that affect the perception of transparencies or use a different alpha curve to draw opacities altogether. This can make the image appear more transparent than it actually is. However, I believe the setting is supposed to correct the alpha channels so that the perceived transparency matches the initial linearity set for the alpha channel in a standard sRGB color space to generate PNG images as one would in Photoshop. For instance, a pixel value of B3 for alpha should display a 70% opacity, as B3 is approximately 70% of FF (255), and it should be rendered as such in OBS this change could have a similar effect in both HDR and non HDR but I'm not really sure if to the same degree. Therefore, enabling the setting by default is a good idea, although it ultimately depends on personal preference and the presence of any transparencies in the PNG image used to create the source.

I also ran clang-format and (hopefully) fixed a memory leak and created a pull request for those changes on your fork.

I tested it and it works just fine Thanks for fixing it. Let me know if I should accept the merge now or if you intend to push the default property for linear alpha here.

univrsal commented 1 year ago

I tested it and it works just fine Thanks for fixing it. Let me know if I should accept the merge now or if you intend to push the default property for linear alpha here.

You can merge the changes I made

univrsal commented 1 year ago

Alright, thank you again for taking the time to investigate and fix this!

anchaides commented 1 year ago

Why is the merge partially verified? is there a way to fix that? :(

univrsal commented 1 year ago

no clue, but it also says that three people authored the changes for some reason