search

mpv-player / mpv

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

[RFC] vo_gpu: remove deprecated options --gamma-factor and --gamma-auto #14369

Open Akemi opened 3 months ago

Akemi commented 3 months ago

--gamma-factor and --gamma-auto were deprecated in 0.35 (ac39661 and 2207236). the latter was only used on macOS. the equivalent client API functionality wasn't officially deprecated.

what i would like to discuss: do we want to remove those options already? if yes, how do we want to remove them? since the client API equivalent wasn't deprecated, removal of the functionality can't be done immediately? do we want to deprecate the client API functionality first in the next release and remove the functionality in one of the releases after the next one? the client API deprecation change should probably be taken separately?

hope i found all the dependent code and removed it properly.

github-actions[bot] commented 3 months ago

Download the artifacts for this pull request:

Windows * [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1606059576.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1606061821.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1606068084.zip)
macOS * [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1606060166.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1606059839.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1606059266.zip)
ruihe774 commented 3 months ago

FWIW I wonder what is the alternative of --gamma-factor? I am using --gamma-factor=1.2239 on macOS to match the color with QuickTime Player.

hooke007 commented 3 months ago

what is the alternative

https://github.com/mpv-player/mpv/pull/10699#issuecomment-1325851500 https://github.com/mpv-player/mpv/issues/11153 But it doesn't give the real answer.

na-na-hi commented 3 months ago

Note that after deprecating --gamma-factor, there is no longer a meaningful way to set gamma anymore. The value of --gamma is essentially a meaningless implementation detail: https://github.com/mpv-player/mpv/blob/71d3f4157b0103354382b431352c0bae98bf04fa/video/csputils.c#L499

I suggest to undeprecate --gamma-factor. Even though it was implemented as a part of something which no longer works, the option still has valid uses.

Dudemanguy commented 3 months ago

I'm not sure --gamma-auto has any use but I agree with @na-na-hi that --gamma-factor is worth keeping. It's just a simple multiplier anyway so it's not like there's great code complexity.

Akemi commented 3 months ago

why was this option deprecated in the first place?

hooke007 commented 3 months ago

https://github.com/mpv-player/mpv/wiki/GPU-Next-vs-GPU

--gamma-factor and --gamma-auto: considered deprecated by design, the new BPC logic handles this much better

Not sure if it was related.

Akemi commented 3 months ago

from the commits and the PR i could only gather "outdated information" and "--gamma-auto not working". though both aren't really a reason to deprecate the options. for the former update the info, for the latter fix it or open an issue (without knowing what exactly is broken, hard to judge).

seems like the initial use case for --gamma-factor is gone, though it still has a more general use case of being the only way to adjust gamma at all (besides video filters via ffmpeg i guess?).

then i am also of the opinion to undeprecate both options.

ruihe774 commented 3 months ago

https://github.com/mpv-player/mpv/wiki/GPU-Next-vs-GPU

--gamma-factor and --gamma-auto: considered deprecated by design, the new BPC logic handles this much better

Not sure if it was related.

I wonder what is "BPC"? I cannot find the word in any other places.

Akemi commented 3 months ago

black point compensation, is my guess. at least that's what i see when working with colour profiles and related.