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
[Android] Implement 'HDR display enable' for ffmpeg decoded streams #16557
Conversation
pVideoPicture->color_range = 0; | ||
pVideoPicture->color_range = m_hints.colorRange == AVCOL_RANGE_JPEG ? 1 : 0; | ||
|
||
CLog::Log(LOGWARNING, "XXX PRI: %d TRC: %d SPC: %d RNG: %d", pVideoPicture->color_primaries, pVideoPicture->color_transfer, pVideoPicture->color_space, (int)pVideoPicture->color_range); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure you want this to be LOGWARNING
?
Looks more like a candidate for LOGDEBUG
to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use C++ style cast
@@ -116,6 +116,7 @@ bool CLinuxRendererGLES::ValidateRenderTarget() | |||
|
|||
bool CLinuxRendererGLES::Configure(const VideoPicture &picture, float fps, unsigned int orientation) | |||
{ | |||
CLog::Log(LOGDEBUG, "LinuxRendererGLES: Configire with HDR metadad: %s", (picture.hasDisplayMetadata && picture.hasLightMetadata) ? "true" : "false"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LinuxRendererGLES: Configure with HDR metadata
@@ -7,12 +7,14 @@ | |||
*/ | |||
|
|||
#include "WinSystemAndroidGLESContext.h" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add back the empty line, https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#7-headers
#include "VideoSyncAndroid.h" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the empty line, https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#7-headers
#include "VideoSyncAndroid.h" | ||
|
||
#include "cores/VideoPlayer/DVDCodecs/Video/DVDVideoCodec.h" | ||
#include "platform/android/activity/XBMCApp.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pVideoPicture->color_range = 0; | ||
pVideoPicture->color_range = m_hints.colorRange == AVCOL_RANGE_JPEG ? 1 : 0; | ||
|
||
CLog::Log(LOGWARNING, "XXX PRI: %d TRC: %d SPC: %d RNG: %d", pVideoPicture->color_primaries, pVideoPicture->color_transfer, pVideoPicture->color_space, (int)pVideoPicture->color_range); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use C++ style cast
Jenkins build this please |
2 similar comments
Jenkins build this please |
Jenkins build this please |
jenkins build this please |
system/settings/settings.xml
Outdated
<condition on="property" name="ishdrdisplay" /> | ||
</dependency> | ||
</dependencies> | ||
<!--requirement>ishdrdisplay</requirement--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is now a setting which is only visible if display supports HDR and let users switch between display HDR and internal GL(ES) shader HDR
Really? With this commented out the setting will always be visible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm, sorry for the noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, I'll remove the comment - leftover from testing
Jenkins build this please |
1 similar comment
Jenkins build this please |
I am not seeing this setting on BRAVIA. Suppose it does not support the Android/EGL HDR APIs. |
Working fine on Shield. |
On BRAVIA: |
@CiNcH83 yes, there has to be a EGL extension. Not sure if we will arrive this extension on devices other than Nvidia Shield, because the Shield is the only one which focusses on Game development, too. Could be that we have a chance for other devices if we use the Vulkan API, but - your HDR videos should look ok if you use our HDR implementation. The display switch to HDR could save some GPU resources, but results should match. |
On my BRAVIA, SW decoding in Kodi isn't feasible anyway. HDR video playback via MediaCodec Surface is fine (and has been fine before). The only problem is the Kodi menu/OSD. When the TV detects an HDR video and switches into the respective HDR mode, there is always a distinct color and luminance shift in OSDs. Artwork is way too dark for example. Haven't checked yet whether this is still the case. |
This PR has broken the RPi4 with Kodi 19 (latest master), which is using mesa.
In the kodi debug log:
Reverting this PR (and also #16579) restores working Kodi with RPi4. |
From @Kwiboo on LibreELEC Slack:
and
|
#. Description of setting with label #13436 "Use display HDR capabilities" | ||
#: system/settings/settings.xml | ||
msgctxt "#36299" | ||
msgid "Switch display into HDR mode if media with HDR information is played.\nIf disabled, HDR information are applied using kodi's internal HDR path" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peak3d typo: kodi's => Kodi's (capital K, please) and a period is missing to end the second sentence.
Description
This PR implements switching HDR capable displays into HDR mode when playing HDR streams using LinuxRendererGLES GPU rendering.
When using this path, our internal HDR implementation is overriden and HDR calulation is done on GPU / display, its compareable to audio passthrough.
Motivation and Context
VP9 profile 2 HDR streams are currently not supported on NVIDIA Shield TV, using this PR its from theory possible to play them. 1080p streams don't play smooth yet, but there are still ptions to optimize the render chain (e.g. YUV scanout instead our own YUV2RGB shader)
How Has This Been Tested?
NVIDIA Shield TV + youtube addon
enable vp9 dash in youtube addon settings
search for 'vp9 hdr' and play one stream.
AFTV 4K (no GPU HDR extension)
HDR setting is not visible and internal HDR pipeline is used
WETEK HUB (SDK version does not support HDR)
HDR setting is not visible and internal HDR pipeline is used
For discussion
Solved: Should there be an settings option to disable this feature and use our shader HDR calulation instead?
There is now a setting which is only visible if display supports HDR and let users switch between display HDR and internal GL(ES) shader HDR
Types of change