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 is too bright and lacks color tones #14106

Closed
skrew opened this issue May 10, 2024 · 20 comments
Closed

HDR is too bright and lacks color tones #14106

skrew opened this issue May 10, 2024 · 20 comments

Comments

@skrew
Copy link

skrew commented May 10, 2024

Hi,

  • mpv 0.38
  • tvOS using gpu-next
  • LibMPV

As you can see in the screenshots, the original source (and i have the same with exo player) are darker and have a blue tint.
Note, i'm using the same video file on exoplayer and MPV

IMAGE 2024-05-10 10:00:09
The good profile, as you can have on Netflix or ExoPlayer

IMAGE 2024-05-10 10:00:05
With MPV

There are no problems in SDR mode

If you need the video, I can send a cut-out version with this scene

@kasper93
Copy link
Contributor

kasper93 commented May 10, 2024

With the amount of information provided, it is impossible to diagnose this issue. Please provide log file and shift+i stats in both cases. And sample file, so we know what we are looking at.

@hooke007
Copy link
Contributor

shift+i

It's tvos. How could he use the keyborad...

@kasper93
Copy link
Contributor

I don't have keyboard on my Android device, yet I can enable stats. Believe or not the technology is there to implement such feature without keyboard.

Also if it is not reproducible with mpv desktop binary it means the issue is on the integration side and should be reported there.

@hooke007
Copy link
Contributor

I don't have keyboard on my Android device,

tvOS is not based on Android...

@skrew
Copy link
Author

skrew commented May 10, 2024

logs: http://www.danstaface.net/mpvlog.txt
video with the problem: http://www.danstaface.net/testhdr.mkv

I'm on mac, and gpu-next is not enabled yet on my build, so i can't test if the problem exists with the mpv binary

@kasper93
Copy link
Contributor

Metadata from decoder are incorrect for this video

2024-05-10 13:48:41.962 [vd] info: Using hardware decoding (videotoolbox).
2024-05-10 13:48:41.962 [vd] v: Decoder format: 3840x1920 [0:1] videotoolbox[p010] bt.709/bt.709/bt.1886/limited/auto CL=uhd crop=3840x1920+0+0

Should be:

[  1.667332]                     vd: Using software decoding.
[  1.667494]                     vd: Decoder format: 3840x2160 yuv420p10 dolbyvision/bt.2020/pq/limited/auto CL=uhd crop=3840x2160+0+0

Needs debugging where params are lost in this case.

@Akemi
Copy link
Member

Akemi commented May 10, 2024

i believe the expectations are completely wrong here?

for one gpu-next and libmpv render (i am assuming it's using the render api, since the macvk context doesn't have a tvos backend) are mutually exclusive.

or they are using code that is not even in master, eg this one #7857.

though in both cases, the user itself is responsible for activating HDR/EDR on Apple platforms, since they manage the layer + view themselves.

gonna quote myself. this is analog to an UIView + CAMetalLayer (- opengl view remark):

TLDR:

  • set wantsExtendedDynamicRangeContent = true on the CAOpenGLLayer
  • set wantsExtendedDynamicRangeOpenGLSurface = true on the view containing the above layer
  • set the proper colour space on the above layer (layer.colorspace =), to the color primaries/transfer characteristics of the VO target (see below --target-trc and --target-prim)
  • set primaries/transfer characteristics --target-trc and --target-prim of the display

first 3 make macOS aware that the window/view/layer is presenting something in HDR/EDR, 3 alone tells macOS what colour space the content is rendered in, 4 tells mpv to what characteristics to render to.

also see the Apple docs for the 3 possible ways as reference. they removed the opengl ones sadly, so one has to transfer a little bit.

[edit]

2024-05-10 13:48:41.291 [vo/gpu-next/vulkan] v: Initializing GPU context 'moltenvk'

yeah this is the code from the not merged closed PR.

@Akemi Akemi closed this as completed May 10, 2024
@skrew
Copy link
Author

skrew commented May 10, 2024

Does this mean that mpv no longer supports iOS / tvOS platforms? GLES has been deprecated for a long time, and the only future left is metal, which is supported via gpu-next / libplacebo / vulkan?

@Akemi
Copy link
Member

Akemi commented May 10, 2024

no, nothing official. we still support those as much as before. a deprecation isn't a removal (yet). i know the feeling of not wanting to use opengl, though that's the only way to do it for now.

the closed not merged PR wasn't rejected and i am in favour of merging it or something similar in the future. i just didn't get to it yet.

see also our Want to work on mpv? issue. we are looking for someone to to work on libmpv to make gpu-next usable with it.

@kasper93
Copy link
Contributor

kasper93 commented May 10, 2024

Does this mean that mpv no longer supports iOS / tvOS platforms? GLES has been deprecated for a long time, and the only future left is metal, which is supported via gpu-next / libplacebo / vulkan?

Contributions are welcome. Since you seem to be building tvOS product based on libmpv library, feel free to contribute and changes to the library itself.

@skrew
Copy link
Author

skrew commented May 10, 2024

Ok, I would like to contribute but little knowledge of C and video / audio in general.

Currently, there are 2 patches for iOS / tvOS:

  • 1 to use moltenvk directly
  • 1 to compile MPV 0.38 on iOS

Is there any particular reason to use libmpv when you can use gpu-next directly?
With these 2 patches, mpv works perfectly on iOS and tvOS (apart from the color issue).

I'll help as best I can (with patches if I can, and debugging/testing).

@Akemi
Copy link
Member

Akemi commented May 10, 2024

with libmpv's render API you (can) driver your own render loop, on the other hand the wid embedding from the mentioned PR the mpv core drives the render loop. both approaches have their pros and cons, depending on the implementation. it's probably a bit much discussing this here though.

i am not sure what patch you mean. though if there is something broken with compiling for iOS/tvOS (is there an open issue?) and there is no PR for it, it would be a good start to open PR to get it fixed.

we should also get #7857 rolling again. there were a few questions that still needed to be sorted out. though in general i believe there is nothing major that speaks against that approach.

@skrew
Copy link
Author

skrew commented May 10, 2024

This is the patch i talked about, if the author is present here he can make a PR ? Otherwise, i will post it myself

Plugins/BuildFFmpeg/patch/libmpv/0002-fix-ios-build-failed-on-0.38.0.patch

@Akemi
Copy link
Member

Akemi commented May 10, 2024

that one is already in master e7b0d6b

@skrew
Copy link
Author

skrew commented May 10, 2024

Oh ok... Why didn't I check before... 🙄😅

with libmpv's render API you (can) driver your own render loop, on the other hand the wid embedding from the mentioned PR the mpv core drives the render loop. both approaches have their pros and cons, depending on the implementation. it's probably a bit much discussing this here though.

Yes, I can see the difference, and it's true that having control over his own code is always an advantage. Personally, I don't see any problem in using wid in the first instance.

@Akemi
Copy link
Member

Akemi commented May 10, 2024

with opengl on macOS the problem was a lot more apparent, because the only viable way of doing opengl was a CAOpenGLLayer and with that you had to drive your own render loop to make it work properly.

it's a lot less problematic with metal and how drawable objects work.

@CounterPillow
Copy link
Contributor

It appears @skrew is using libmpv for a commercial player that makes no effort to disclose the used libmpv and ffmpeg sources, as is required per the LGPL. This means he is engaging in large-scale software piracy for commercial gain.

@skrew
Copy link
Author

skrew commented May 10, 2024

Like that @CounterPillow ?

Simulator Screenshot - Apple TV 4K (3rd generation) (at 1080p) - 2024-05-10 at 22 43 55

MPV & MPVLib are mentioned, the others players too, as well as the metadata sources (TMDB & TVDB).

If you want more disclosures in other parts, tell me (just before i'm blocking you, because i don't like your attitude)

@CounterPillow
Copy link
Contributor

Read the LGPL license terms, simply mentioning you're using libmpv is not enough. It is funny you're complaining about my attitude when you're the one freeloading off other people's work, and then being pissy when it doesn't do what you want it to do.

@richardpl
Copy link
Contributor

Ah, Plex 2.0 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants