mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
27.71k stars 2.86k forks source link

HDR is too bright and lacks color tones #14106

Closed skrew closed 3 months ago

skrew commented 3 months ago

Hi,

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 commented 3 months ago

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 commented 3 months ago

shift+i

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

kasper93 commented 3 months ago

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 commented 3 months ago

I don't have keyboard on my Android device,

tvOS is not based on Android...

skrew commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 https://github.com/mpv-player/mpv/pull/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.

skrew commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

Currently, there are 2 patches for iOS / tvOS:

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

that one is already in master https://github.com/mpv-player/mpv/commit/e7b0d6b38b3d80f57f2c585ca9b4ab1cc6e213b0

skrew commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

Ah, Plex 2.0 ?