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

App does not work with multiple identical monitors #49

Closed
drewcotten opened this issue Sep 25, 2018 · 78 comments · Fixed by #550
Closed

App does not work with multiple identical monitors #49

drewcotten opened this issue Sep 25, 2018 · 78 comments · Fixed by #550
Assignees
Labels
Status: Done Work on this issue is complete. Will be available on next release Type: Bug Issue is a bug (e.g. Crash, …)

Comments

@drewcotten
Copy link

drewcotten commented Sep 25, 2018

Unexpected behavior w/ 2 identical BenQ BL3201PH monitors connected via DisplayPort on a GTX 980ti, running 10.13.6. App is only able to control DDC on one of the two monitors. When the working monitor is the primary "Mac" monitor as chose in Display Preferences in the Arrangement tab.

Disabling the primary monitor in MonitorControl settings does not fix this issue. When I disable the "main" working monitor in Monitor Control preferences, adjusting the other monitor brightness causes the main monitor brightness to then change. Same applies for the brightness sliders in menubar. Has anyone else experienced this?

I've tried changing the system monitor names with no luck, no change in name in the MonitorControl app. Will keep trying other options.

screen shot 2018-09-24 at 10 14 41 pm

screen shot 2018-09-24 at 9 14 27 pm

@KrissHeo
Copy link

I have same issue. two identical monitor (alphascan 5K27), connected to rx480(egpu)

@drewcotten
Copy link
Author

drewcotten commented Sep 25, 2018

It appears most DDC control apps on Github have this same issue. It must be a root setting in ddctl (which all Mac apps that control via DDC use as far as I can tell) that is causing the issue - https://github.com/kfix/ddcctl

Edit: In fact, this is an unresolved issue on the ddcctl Github. If we can fix there, we should be able to fix here. See: kfix/ddcctl#17

@the0neyouseek the0neyouseek added Priority: Minor Issue is minor (e.g. Monitor or cable type specific…) Status: Blocked Work on this issue is blocked (reason in comment) Type: Bug Issue is a bug (e.g. Crash, …) labels May 23, 2019
@the0neyouseek
Copy link
Member

Hi, is this still an issue with the latest version ?

@esetnik
Copy link

esetnik commented Jun 3, 2019

I am able to control brightness on multiple monitors in latest version.

@drewcotten
Copy link
Author

I am able to control brightness on multiple monitors in latest version.

This issue does not relate to multiple monitors - it relates to identical monitors. Unfortunately, in the latest version of monitor control, I still have this issue with my two identical displays. I believe the root problem is with ddcctl and further down the line Apple's APIs. I would recommend anyone having this issue post here kfix/ddcctl#17

@esetnik
Copy link

esetnik commented Jun 4, 2019

Ok sorry for the lack of helpful feedback on my part. In case it matters I am using dual identical P2715Q monitors without any issue.

@drewcotten
Copy link
Author

drewcotten commented Jun 4, 2019 via email

@geoffmyers
Copy link

I am experiencing the same issue with two Samsung U28E590 monitors both connected via DisplayPort to a Radeon RX 580. The brightness controls only work for one monitor but not the other.

@leo150
Copy link

leo150 commented Jun 13, 2019

Same for me with two BenQ GW2760HL. It doesn't look like there is any progress on kfix/ddcctl/issues/17. Is it possible to override vendor or model for a specific monitor to trick ddcctl?

@JoniVR
Copy link
Member

JoniVR commented Jun 13, 2019

ddcctl is no longer used internally. No point to link to that specific issue anymore since it's already been linked multiple times.

We now use DDC.swift. But I think the source code is based on ddcctl so it's probably the same underlying issue.

@kevinjohncutler
Copy link

Same issue with dual viewsonic vp2780-4k monitors. Can only adjust the second display by turning the primary one off. On the latest version.

@jasonbarrows
Copy link

I have just noticed the very same issue using 2 x BenQ PD2700Q monitors using integrated Intel HD4000 graphics. Audio over DisplayPort is only possible via one out of the two ports. I don't know if that will have something to do with it.

@dmarin
Copy link

dmarin commented Nov 19, 2019

Same issue, 2 hp monitors. I can see the 2 devices in "monitorControl", but it does not matter which one I choose it always changes the settings for the "main" one

@leo150
Copy link

leo150 commented Nov 20, 2019

We need a hero with carthage experience.

@sndwch
Copy link

sndwch commented May 27, 2020

Same issue, but this app works for brightness of my identical displays: https://github.com/fnesveda/ExternalDisplayBrightness/

@leo150
Copy link

leo150 commented May 28, 2020

They did a fix for the identical monitors: commit

@JoniVR JoniVR changed the title App does not work with multiple monitors App does not work with multiple identical monitors May 31, 2020
@leo150
Copy link

leo150 commented Jun 6, 2020

If anyone's interested in a solution, I've adopted the fix mentioned above to DDC framework which is used in MonitorControl. It works with my identical BENQ displays, didn't test with anything else. Here's the fork.

If you want to try it out, update the Cartfile.resolved to point on github "leo150/DDC.swift" "be2d2a3f5662b3d529e5ecd13b9c97fadec46d3b"

@JoniVR
Copy link
Member

JoniVR commented Jun 6, 2020

I can provide a full test build with your fork included soon in here if you want. That way if more people can confirm it works for them you can open a PR in DDC.swift.

@ndrwstn
Copy link

ndrwstn commented Jun 6, 2020

I can provide a full test build with your fork included soon in here if you want. That way if more people can confirm it works for them you can open a PR in DDC.swift.

I would be interested in trying this; I have identical BenQ PD3220Us that are exhibiting this issue.

@waydabber waydabber added Help wanted Issue would benefit from the help of a dev, anyone can pick it up and removed Status: Feedback needed from issue reporter Waiting for issue reporter to provide feedback (see comments) labels Aug 24, 2021
@waydabber
Copy link
Member

@iRayanKhan, MC refreshes the display if it receives a call from the OS using CGDisplayRegisterReconfigurationCallback() - the fact that the app is continuously reconfiguring (if I understand correctly) would mean that the OS is constantly doing something with the display configuration (but not as frequently as 2 seconds because within a 2 second window the app is still waiting if there are further changes as macOS tends to send several callbacks during a single change). So it might be that MC itself is forcing the reconfiguration. But MC should not do anything actively with the display except read its settings via DDC. If this forces a reconfiguration (it should not), then my theory is that one of the displays might not like DDC reads and behaves badly because of this and disconnects/reconnects briefly which triggers a reconfiguration. You can make it stop doing this if you enable Advanced in General and then set the polling mode to 'None' - this still enables DDC writes so it should not impair usability. Does that fix the issue?

@thajcak - from the screenshot as far as I see the two displays have a separate DirectDisplay IDs so they should be controlled independently. I don't clearly see what the problem is. The fact that the two displays have the same name should not matter since the app works using IDs not names. Reading the earlier remarks, this might be an Intel issue (as I see from the IDs, since such IDs are never given by the OS on M1) and might have something to do with DDC.Swift (which is an external library the app uses for Intel control). Unfortunatelly I don't know in depth how DDC.Swift works (I was working on the M1 control library) as I don't have an Intel machine (and two identical displays), so its a bit hard to debug this problem as of now. It would be great if somebody with an Intel setup and two displays would volunteer to understand what is happening exactly.

I added a "Help wanted" tag to this thread. (@JoniVR - is there a better way to raise awareness to an issue in the community?)

@glumb
Copy link

glumb commented Aug 24, 2021

Also for me the same issue persists. Unfortunately I can not volunteer right now since I'll be on vacation without my external monitors 😄

image

image

@thajcak
Copy link

thajcak commented Aug 24, 2021

Yea, I'm not sure what's going on either. @waydabber I'm happy to help out with testing anything you need but I won't have time to dig into code.

I'll just throw out there that there's an app called Lunar that does work properly on my system as a data point. It's closed source so it's not very helpful other than knowing that it is a solvable problem.

@waydabber
Copy link
Member

Yes, I know Lunar which is a great app who's developer @alin23 became a buddy of sort of mine, but it uses a completely different code for DDC control both for Intel and M1.

I am sure this is not an issue that can't be fixed, I just don't have the hardware to debug this one so it doesn't make sense for me to try to figure out what is happening.

Anyway, to make some progress, I went through the relevant code quickly in MonitorControl and it does not seem to use the Display name for anything other than superficial purposes (showing stuff on the GUI) - from the earliest moment it uses the display ID provided by CoreGraphics DisplayID (which is an unique identifier, in case of Intel macs its a long number, for M1 macs its a short one) for identification so the two displays are not mixed up somehow. It seems like the problem is in DDC.swift which is an external library used for Intel communication. I followed that trail and I think the issue is in DDC.servicePort(from displayId: CGDirectDisplayID, detectUnitNumber: Bool) where there is a service matching logic that might not be able to distinguish between similarly named displays (since all the identifiers it uses are ones that might be exaclty similar for two different displays, including display serial number which is not really too unique for most display makers). There is a similar challange for M1 but since that logic was written by me ;) I recognized this trap early on and made sure that such mixups won't happen (not that it is anything beyond a theoretical possibility since M1 macs can control a single external display as of now - even including M1 due to the HDMI issue with the minis). So anyway, there seems to be an approach problem in DDC.swift as its matching logic is designed to work attached to an instantiated object representing a display, but it does not know about other displays - so it is unaware if a service port which is found to be matching to a display is already taken by an other display - this results in the situation where in case of totally similar displays the same service port will be associated to each display (precisely because of this the M1 matching logic is an external, static solution that matches all service slots to possible displays using a score based fuzzy logic thing and if there are two similar displays, never two display gets associated to the same service, since the solution it is aware which one is taken and which one is free). A more serious rework might be needed to fix this (or maybe we should replace DDC.Swift and use our own logic). I could try to somehow fix it but without an Intel Mac it would be impossible for me to see what's happening and this is not something I can do blindly. :( I think ddcctl might have the same issue (as I read above somewhere) precisely because it works with individual displays on the fly just as it is the case with DDC.swift.

@waydabber
Copy link
Member

A quick solution might be that since MC can recognize the fact that two or more displays have exaclty the same characteristics, it can pass this information along to DDC.swift as an orchestrator, signifying which display is regarded as first, which one second etc. DDC.swift could be modified in a way that this info is passed along the required layers (I counted about 3 or 4) to the service matching logic, where it would be factored in during matching - if the display is the second one, then the first service port match is skipped and the second one is served, if the display is the third one, then the first two are skipped etc.

But this requires modifying DDC.Swift (which doesn't seem to be maintained nowadays) or moving it into MonitorControl proper (probably this is the better way). Still, somebody with an Intel machine should do this I think.

@alin23
Copy link

alin23 commented Aug 24, 2021

Yea, I'm not sure what's going on either. @waydabber I'm happy to help out with testing anything you need but I won't have time to dig into code.

I'll just throw out there that there's an app called Lunar that does work properly on my system as a data point. It's closed source so it's not very helpful other than knowing that it is a solvable problem.

Hi @thajcak !

More than 90% of Lunar (including the display detection part) is still open source here: https://github.com/alin23/Lunar

Just leaving this here so we don't mislead people. I'd be happy to help with relevant parts from the Lunar code that solve this same monitor model differentiation problem.

For the Intel Macs, the solution is very simple. Instead of doing error prone EDID matching to get the framebuffer, it's better to use the private CGSServiceForDisplayNumber function: https://github.com/alin23/Lunar/blob/04df526af58d4acb161d06f8d8c3923e4d27164c/Lunar/DDC/DDC.c#L158-L167

For M1, the problem is a lot more complex and @waydabber knows it very well. But last time I checked the ioreg matching code of MC, everything looked well.

Is this happening only on Intel or on M1 as well?

@waydabber
Copy link
Member

waydabber commented Aug 24, 2021

Hi @alin23 - this is an Intel only problem. MC uses DDC.swift for Intel which is an external library. DDC.swift does not use CGSServiceForDisplayNumber for sure but instead a solution that is similar to EDID matching (if you look at its code, you'll see that it is searching for the service port using model id, vendor id, serial number match). I think this approach would still work if it was implemented with some orchestration in mind in light of the possibility of multiple identical displays. I don't know (didn't look) whether ddcctl uses the same logic or did use it in only in the past (DDC.swift was modelled after it) but maybe has an updated logic now.

Anyway, if there is such a great thing as CGSServiceForDisplayNumber for Intel (lol!) then we should definitely use that (am I seeing correctly that this a recent - 2 months old - development in Lunar as well?). But that requires disengaging MC from DDC.swift (which I think is a good idea) and still somebody with an access to an Intel mac should do it as I can't even try out if what I am doing works or not.

UPDATE: I quickly checked ddcctl out, it also does not use CGSServiceForDisplayNumber() but has a logic what you would describe as EDID matching. :)

@alin23
Copy link

alin23 commented Aug 24, 2021

The EDID matching started from ddcctl and survived into Lunar and DDC.swift but no matter what you try, there will always be cases where the EDID data is identical between monitors.

Like in the AVService matching logic, you could just assign the framebuffers randomly for identical monitors but you wouldn't have a persistent ID to store settings per monitor in that case.

The CGS API is indeed a very recent addition and has been worked perfectly so far. I recommend it.

It's stolen from a different branch of ddcctl: kfix/ddcctl@c9d8fb7

@waydabber
Copy link
Member

Nice find indeed! I say MC should... appropriate this then as well. :) When I'm back from holiday, I'll probably make a test version that... purloins DDC.swift (in the name of democracy or something) and replaces the offending logic with this single good looking private API call. I won't be able to test it without an Intel Mac but since I know how noble of heart you are @alin23, I am sure you'll help testing it with me at least at a basic level (which might be awkward since theoretically MC is a competitor to Lunar but this is for science and also good PR), and then let folks here with Intel macs and similar displays do the rest of testing. What do you say? ;)

@alin23
Copy link

alin23 commented Aug 24, 2021

Sure, I still have an Intel MacBook around that I keep for testing stuff like this. I'd be glad to help 😊

@waydabber waydabber removed the Help wanted Issue would benefit from the help of a dev, anyone can pick it up label Aug 25, 2021
@waydabber waydabber added this to the v3.1.0 milestone Aug 26, 2021
@waydabber
Copy link
Member

waydabber commented Aug 30, 2021

Here is an unsigned test build that uses CGSServiceForDisplayNumber to acquire the proper service port and should not confuse identical displays - if somebody would like to try it out (the source of this build is here). Thank you!

@waydabber
Copy link
Member

Also, I'd like to thank @alin23 for trying out this code and confirming that it indeed uses CGSServiceForDisplayNumber as intended on his Intel Mac.

@thajcak
Copy link

thajcak commented Aug 30, 2021

Here is an unsigned test build that uses CGSServiceForDisplayNumber to acquire the proper service port and should not be confuse identical displays - if somebody would like to try it out (the source of this build is here). Thank you!

I appreciate the the attempt, this works for me!

@waydabber waydabber added Status: In progress Issue currently being worked on Status: Done Work on this issue is complete. Will be available on next release and removed Status: In progress Issue currently being worked on labels Aug 31, 2021
@waydabber waydabber added this to Needs triage in Bug tracking via automation Sep 1, 2021
@waydabber waydabber linked a pull request Sep 1, 2021 that will close this issue
@waydabber waydabber removed this from the v3.1.0 milestone Sep 1, 2021
Bug tracking automation moved this from Needs triage to Closed Sep 1, 2021
waydabber added a commit that referenced this issue Sep 1, 2021
- Fixed not working after sleep mode for some on Apple Silicon Not working after sleep mode #530
- Fixed some LG and Samsung displays having problems with Mute (improved 'Enable Mute DDC command') - LG Monitor: have to unmute manually after muting #170
- Fixed app not working with multiple identical monitors on Intel - App does not work with multiple identical monitors #49
- Added 'Safe Mode' option - pressing the Shift key during startup resets preferences and disables DDC read.
- Upon first start if DDC is unreadable, default brightness/volume/contrast values are now set to a sensible 75% instead of 0%
- DDC write commands are issued twice on Intel (as it already was on Arm64) to improve stability on some setups.
- Make sure DDC communications don't happen in parallel when both slider menu and keyboard is used (this might have caused problems with some docks with multiple display outputs).
- Fixed volume control feedback audio (clicking sound) during key repeat (it should play on keyup only as this is the macOS standard).
- Fixed duplication of volume control feedback audio if there are multiple external displays and 'Change... for all screens' is enabled.
- Internal DDC library for Intel (based on the work of reitermarkus)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Done Work on this issue is complete. Will be available on next release Type: Bug Issue is a bug (e.g. Crash, …)
Projects
Development

Successfully merging a pull request may close this issue.