FedeDP / Clightd

A linux bus interface that lets you change screen brightness, compute captured webcam frames brightness and change screen temperature.
GNU General Public License v3.0
82 stars 10 forks source link

modify wl screen implementation for multiple outputs #96

Closed tpeacock19 closed 1 year ago

tpeacock19 commented 1 year ago

This commit updates the wayland screen also provides an ability to get screen brightness per output. It currently averages the brightness but can be useful if we allow for per-output screen brightness modifications. In order to accomplish the above wlr-screencopy-unstable-v1 protocol was updated and wlr-output-management-unstable-v1 was added.

It should be much simpler than my previous PR.

tpeacock19 commented 1 year ago

One thing to note, It seems that both this, and the previous implementation of the screen module only takes a small portion of the left side of the screen when calculating brightness. Do you experience anything similar with the X11 implementation?

For example, these two images will show drastically different brightnesses:

2023-01-08T18:30:15,431624632-07:00 2023-01-08T18:30:18,374191027-07:00

FedeDP commented 1 year ago

Hi! Sorry for the long time no see! I can see the same issue in the xorg impl :/ Most probably, the issue lies in the rgb_frame_brightness algo. I am going to give it a look! Thanks for this btw!

FedeDP commented 1 year ago

97 should fix the issue (it fixed the issue on xorg for me ;) )

tpeacock19 commented 1 year ago

I'm taking a look at this today. I'll update this asap.

Also, is https://github.com/FedeDP/Clightd/pull/95/files#diff-585db1c269ab2cb67ea2d2ccaa2439cbbb4e5f40d0f5254c297d90d7cd5d4159R111 a fix worth porting? Mind to port it in this PR or opening a new one?

I can make a new PR, just to keep things more explicit.

tpeacock19 commented 1 year ago

97 should fix the issue (it fixed the issue on xorg for me ;) )

I'll need to update this PR to follow this change. What is the bytes_per_line that you replaced the width with?

FedeDP commented 1 year ago

I think the wl implementation is already ok. Bytes_per_line is the ximage field for image stride!

FedeDP commented 1 year ago

(the bug was in the algorithm, but xorg impl had also that bug, ie: it was passing the wrong param as stride)

tpeacock19 commented 1 year ago

okay great, I've updated this to with your suggestions.

FedeDP commented 1 year ago

Were you able to test the fix in #97 on wayland? Did it work fine?

Btw i assume this patch is working fine; can you test a run with valgrind tool? I use: valgrind --tool=memcheck --leak-check=full --track-origins=yes -v ;)

tpeacock19 commented 1 year ago

I did test it with the most recent commits and it works as expected. I'm not familiar with valgrind but it does not seem to work.

# env WAYLAND_DEBUG=1 CLIGHTD_PIPEWIRE_RUNTIME_DIR=/run/user/1000/ valgrind --tool=memcheck --leak-check=full --track-origins=yes -v ./build/clightd
==214954== Memcheck, a memory error detector
==214954== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==214954== Using Valgrind-3.19.0-8d3c8034b8-20220411 and LibVEX; rerun with -h for copyright info
==214954== Command: ./build/clightd
==214954==
--214954-- Valgrind options:
--214954--    --tool=memcheck
--214954--    --leak-check=full
--214954--    --track-origins=yes
--214954--    -v
--214954-- Contents of /proc/version:
--214954--   Linux version 6.1.12-zen1-1-zen (linux-zen@archlinux) (gcc (GCC) 12.2.1 20230201, GNU ld (GNU Binutils) 2.40) #1 ZEN SMP PREEMPT_DYNAMIC Tue, 14 Feb 2023 22:08:11 +0000
--214954--
--214954-- Arch and hwcaps: AMD64, LittleEndian, amd64-cx16-lzcnt-rdtscp-sse3-ssse3-avx-avx2-bmi-f16c-rdrand-rdseed
--214954-- Page sizes: currently 4096, max supported 4096
--214954-- Valgrind library directory: /usr/lib/valgrind
--214954-- Reading syms from /home/user/src/archlinux/Clightd/build/clightd
--214954-- Reading syms from /usr/lib/ld-linux-x86-64.so.2
--214954--    object doesn't have a symbol table

valgrind:  Fatal error at startup: a function redirection
valgrind:  which is mandatory for this platform-tool combination
valgrind:  cannot be set up.  Details of the redirection are:
valgrind:
valgrind:  A must-be-redirected function
valgrind:  whose name matches the pattern:      strlen
valgrind:  in an object with soname matching:   ld-linux-x86-64.so.2
valgrind:  was not found whilst processing
valgrind:  symbols from the object with soname: ld-linux-x86-64.so.2
valgrind:
valgrind:  Possible fixes: (1, short term): install glibc's debuginfo
valgrind:  package on this machine.  (2, longer term): ask the packagers
valgrind:  for your Linux distribution to please in future ship a non-
valgrind:  stripped ld.so (or whatever the dynamic linker .so is called)
valgrind:  that exports the above-named function using the standard
valgrind:  calling conventions for this platform.  The package you need
valgrind:  to install for fix (1) is called
valgrind:
valgrind:    On Debian, Ubuntu:                 libc6-dbg
valgrind:    On SuSE, openSuSE, Fedora, RHEL:   glibc-debuginfo
valgrind:
valgrind:  Note that if you are debugging a 32 bit process on a
valgrind:  64 bit system, you will need a corresponding 32 bit debuginfo
valgrind:  package (e.g. libc6-dbg:i386).
valgrind:
valgrind:  Cannot continue -- exiting now.  Sorry.