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

linux-pipewire: Fix 10- and 16-bit captures #11207

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PancakeTAS
Copy link

Description

Tonemap textures from non-linear sRGB to linear sRGB when in 10-bit or higher color format and update texture format accordingly

Motivation and Context

Fix white-tint when using a pipewire capture in a 10-bit or 16-bit color format regardless of monitor source bitdepth

How Has This Been Tested?

Before/After in NV12, P010 and P416 color formats on both 10-bit and 8-bit monitors. Confirmed to work both in Preview (no white tint observed) and custom FFmpeg render (10-bit gradient is captured)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue Linux Affects Linux labels Aug 28, 2024
@PancakeTAS
Copy link
Author

not entirely sure how that 'g' just disappeared.. will fix this tomorrow

@@ -1373,6 +1374,13 @@ void obs_pipewire_stream_video_render(obs_pipewire_stream *obs_pw_stream,
gs_blend_state_push();
gs_blend_function(GS_BLEND_ONE, GS_BLEND_INVSRCALPHA);

enum gs_color_format format = s_get_color_space() == GS_CS_SRGB
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this set OBS_SOURCE_SRGB capabilities, and and then configure framebuffers as srgb so decompression is done in hardware, you can look at xshm for an example.

Also the assumption that 10bit formats are srgb seems pretty flawed based on windows examples of how programs actually work. Please check if pipewire reports colorspace information so that we can make an informed choice here or if they are lack this.

Copy link
Author

@PancakeTAS PancakeTAS Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've implemented the SRGB capabilities and adjusted the code, however I do still have a question.

PipeWire returns SPA_VIDEO_COLOR_MATRIX_ as color matrix, which in video_colorspace_from_spa_color_matrix in pipewire.c:614 is converted into an enum either DEFAULT, 601 or 709. The HDR formats here are simply being passed through as DEFAULT, however seeing how the rest of the code doesn't seem to support HDR anyways I will glance over this detail.
Now what I'm asking is, how do I render textures in this color format. Assuming 10-bit formats are sRGB seems flawed, I agree, however I don't know how I'm suppose to handle 601 and 709 textures. (Do I have to tonemap them from their source format into the current obs space? Which effect technique would I use for this?)

Copy link
Collaborator

@kkartaltepe kkartaltepe Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to bring the HDR textures into linear space, no tone mapping needs to be done in the plugin. For non-srgb source there are some more draw techniques for common color spaces you can look at the windows game capture plugin for a more complete HDR implementation.

--- edit

Actually we do use a lower precision space for linear so you do need to tonemap but windows capture has the examples for this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be a little bit confused, but is this not exactly what I am doing?
The PipeWire source texture is always going to be either sRGB, Rec. 601 or Rec. 709. Other color spaces are not supported by the rest of the code.. I don't even think any compositor supports HDR.

If I'm rendering sRGB, Rec. 601 or Rec. 709, the technique always stays the same, no? No tonemapping occurs, or at least I could find zero implementations of it in the entirety of OBS source code.

I also checked rendering to an HDR color space (such as Rec. 2100). The source turns desaturated, which is expected. The windows capture does this as well.

So I'm a little confused on what exactly you want me to add to my code.

(I looked at the windows capture and it seems most of whats different compared to my implementation is actually rendering from and to HDR color spaces)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kkartaltepe
got any updates on this?

Copy link
Collaborator

@kkartaltepe kkartaltepe Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I thought you had wanted to try and extend this to supporting the other spaces. As it is seems ok, the regular draw technique should work now that you have added srgb tagging to the images maybe confirm that and we can remove the extra technique handling too.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I thought you had wanted to try and extend this to supporting the other spaces. As it is seems ok, the regular draw technique should work now that you have added srgb tagging to the images maybe confirm that and we can remove the extra technique handling too.

It's always using DrawSrgbDecompress, there aren't any other techniques being used. Replacing DrawSrgbDecompress with Draw doesn't work, if that's what you meant, the textures get the white tint again.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kkartaltepe Mind taking a look at this again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue Linux Affects Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants