8336277: Colors are incorrect when playing H.265/HEVC on Windows 11 #1525
Conversation
|
👋 Welcome back almatvee! A progress list of the required criteria for merging this PR into |
|
@sashamatveev This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 17 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
This might sound silly, but these formats seem to differ by the way color bits are laid out in memory - would it be possible to write a utility that does the direct conversion instead of calling two conversion methods? |
NV12 and IYUV are different in memory layout only, but P010 is 10-bit per pixel and NV12 is 8-bit per pixel. I filed enhancement request https://bugs.openjdk.org/browse/JDK-8337686 to support P010 and NV12 directly by Graphics. I think it is better approach, then doing conversion in Media. For now this approach is good enough until JDK-8337686 is implemented. |
thank you for filing the RFE. Should JDK-8337686 explicitly specify that the double conversion introduced in this PR be replaced? |
|
/reviewers 2 |
|
@kevinrushforth |
I think this issue/PR should specify it. I just updated description for this PR that color conversion is temporary solution. |
The main reason to mention what needs to be corrected is to help the person who will be working on JDK-8337686 many years from now ;-) |
|
@arapte or @andy-goryachev-oracle can one of you be the second reviewer? |
Done. |
|
not yet a review. with this change, I see an exception in com.sun.media.jfxmediaimpl.NativeMediaPlayer:1530 because VideoResolution is being constructed with width=0 and height=0 edit: on windows, this message does not appear. instead, I see the slider jumping back to the original position momentarily. this differs from macOS/master where the slider position jumps momentarily to a position nearby. |
|
on my windows 11, I hear the audio but see no video (with this PR). am I missing any codecs? |
|
Anything seen on macOS is unrelated to this PR. This PR only modifies Windows-specific platform code. |
It should show the video as well. The expected behavior is that the video is shown with incorrect colors without this fix and shown with correct colors with the fix. I wouldn't expect any specific codec needed, but @sashamatveev can confirm. |
|
rebuilt the code with Still see no video (but I can hear audio). What gives? |
|
Apparently, one needs to pay Microsoft $0.99 for HEVC codecs |
|
/integrate |
|
Going to push as commit 635a09c.
Your commit was automatically rebased without conflicts. |
|
@sashamatveev Pushed as commit 635a09c. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1525/head:pull/1525$ git checkout pull/1525Update a local copy of the PR:
$ git checkout pull/1525$ git pull https://git.openjdk.org/jfx.git pull/1525/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1525View PR using the GUI difftool:
$ git pr show -t 1525Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1525.diff
Webrev
Link to Webrev Comment