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

HDR support, fixes #255 #511

Merged
merged 2 commits into from
Jan 3, 2022
Merged

HDR support, fixes #255 #511

merged 2 commits into from
Jan 3, 2022

Conversation

romkal
Copy link

@romkal romkal commented Dec 28, 2021

This should fix the #255. I observed the same problem and although this is a dirty workaround, it works well enough. For sure better than the flashing.

The build system assumes OpenSSL 1.1. Installation of 3.0 alone won't work.
@psieg-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

1 similar comment
@psieg-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

It seems that on HDR once in a while we get
DXGI_FORMAT_R16G16B16A16_FLOAT even if we only should get regular RGB.

This happens when the duplication has an update ONLY on the mouse
position and not on the actual image. It seems that then the system
doesn't bother to make any buffer conversions and we can use the old
image we acquired before.

The lack of conversion messes up the color, as we append -1 to the buffer and it effects
with white flashes once in a while.
@romkal
Copy link
Author

romkal commented Dec 28, 2021

Actually now it seems to me like a bug in the OutputDuplication as it seems it doesn't do any format conversions if the only update after getting a new frame is the cursor position. I couldn't find it documented anywhere, but...

Once I added the code that checks for LastPresentTime == 0, we can always recognize the format and never get the infamous DXGI_FORMAT_R16G16B16A16_FLOAT .

@romkal
Copy link
Author

romkal commented Jan 3, 2022

@psieg
Copy link
Owner

psieg commented Jan 3, 2022

Thanks for spotting this! It seems the problem is simpler than it looked :)

@psieg psieg merged commit c6b0a43 into psieg:master Jan 3, 2022
@romkal romkal deleted the hdr branch January 3, 2022 23:45
@romkal romkal changed the title HDR support HDR support, fixes #255 Jan 3, 2022
@romkal
Copy link
Author

romkal commented Jan 4, 2022

I've just realized I have no idea how to mark this pull request as fixing #255 ...

@FluffyDiscord
Copy link

Oh my god this is huge, can anyone build a release please?

@psieg
Copy link
Owner

psieg commented Jan 8, 2022

@romkal you did associate it by mentioning the issue number in the PR description.

I'll make a release today.

@Benik3
Copy link

Benik3 commented Jan 8, 2022

WOW, thank you so much! I already had prepared solution with converting it with QT (because QT6 has finally ability to make HDR pictures).
In my opinion it's obvious bug in DDUpl, I already reported it to microsoft, nothing changed:
https://docs.microsoft.com/en-us/answers/questions/179015/duplicateoutput-doesn39t-convert-format.html?childToView=181497#comment-181497

@romkal
Copy link
Author

romkal commented Jan 10, 2022

You might argue that this is an optimization. If nothing changed, why to even do the work of converting the buffer.
Still should be clearly documented and not require almost a salt of luck to figure it out.

@BtbN
Copy link

BtbN commented Feb 9, 2024

I just found this one by chance, and it FINALLY made me realize what on earth is going wrong with ddagrab in FFmpeg, so thanks for that!
Related patch: FFmpeg/FFmpeg@21c6d12
(I picked a bit of a different approach, to not miss pointer updates, and copy the entire texture to a buffer to use in those cases...)

I had legitimately spent months wondering what the hell is going on, and why ddagrab sometimes randomly decides to change the output format to one not even supplied to the initial list of supported formats...
Yeah, I totally agree that this is most definitely a bug on the Windows-Side of things. It seems to skip format conversion on frames without an update.

This whole thing makes me wonder if one isn't even supposed to use the desktop resource/texture on those instances, and the API considers it invalid. Meaning it works for 8 bit by pure chance for a whole lot of applications.

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.

6 participants