WinSystemAmlogic: read Dolby Vision support on display events - #74
WinSystemAmlogic: read Dolby Vision support on display events#74matthane wants to merge 4 commits into
Conversation
| } | ||
|
|
||
| // missing or empty dv_cap counts as supported | ||
| bool dv_supported = true; |
There was a problem hiding this comment.
Now you default to DV support even when dv_cap path does not exist?
There was a problem hiding this comment.
That is the behavior carried over from before. The old aml_display_support_dv() left its static at -1 when the node was missing, and -1 returned as bool is true, so a missing dv_cap already counted as supported. I kept it so the only change here is the verdict staying live.
There was a problem hiding this comment.
Then this was a bug, please fix it for the rework.
There was a problem hiding this comment.
Fixed in the rework. The verdict now derives from the parsed capabilities, so a missing or unreadable dv_cap counts as unsupported, and the flag starts out false until the first real read.
| else if (valstr.find("2160p60hz: 1") != std::string::npos) | ||
| caps.SetDolbyVision4k60(); | ||
|
|
||
| dv_supported = valstr.find("The Rx don't support DolbyVision") == std::string::npos; |
There was a problem hiding this comment.
Use directly dv_supported = caps.SupportsDolbyVision() != DOLBYVISION_TYPE_NONE?
There was a problem hiding this comment.
From what I can see they would come out the same on the 5.15 kernel, since a block that parses correctly always prints one of the 2160p lines. But that line reads more like a formatter convention than the verdict, for V2 blocks the kernel prints 2160p30 as a floor even when the sink may not do 2160p at all, so I kept the deny line since it is the kernel's own verdict and it keeps this the exact old check made live. If you prefer the caps check I can change it.
There was a problem hiding this comment.
Yes, it should match kernel. I think 2160p30 is set as default anyway if DV support. It's only about the minimum 597MHz clock need for 60hz.
There was a problem hiding this comment.
Changed to the caps check. The clock detail matches what I found in the kernel source, so the deny line is gone entirely.
aml_display_support_dv() cached its dv_cap read for process lifetime, so a sink swap could leave the wrong verdict until restart. Derive it in UpdateHDRCapabilities(), which rereads dv_cap on init and on hotplug. A missing or unreadable dv_cap now counts as unsupported.
aml_dolby_vision_enabled() and aml_convert_to_dv_by_vs_engine() cached their device and display checks on first call, so they never picked up a display DV verdict change.
Visibility was decided once at boot and never revisited, so a sink change could leave the DV settings wrongly shown or hidden for the session. Re-evaluate it on display events from the live DV verdict. The settings chained by enable dependencies stay device-keyed so a disabled row always has its cause on screen.
2a8f6cb to
23a423a
Compare
|
Sorry, this PR just don't work as it should. I boot with DV display, disconnect HDMI. Changed TV (EDID) to a non DV TV and kodi shows this on HDMI reconnect: I also think to update |
The not-ready early return also skipped the capability refresh, so a disconnect or a swallowed reconnect left the DV verdict and the settings frozen on the previous sink. The sysfs nodes are live state, so the read stays current no matter who raised the event, even when the mode switch has to wait.
|
The capability refresh sat below the not-ready early return, so a disconnect or a not-ready reconnect skipped it with no retry and the verdict stayed frozen on the previous sink. The new commit moves the refresh above that gate. Here that fixes it, pulling the cable now drops the verdict and hides the sink settings right away, and a reconnect brings them back with DV playback working without a restart. On the DRM property, from what I can see it reads the same live kernel state as the sysfs node, so the ordering comes out the same. I can move the reads over in a follow up if you prefer it for consistency. |
|
We should consider the new commit 26c571e as a bug fix to #73 rather than part of this series. The not-ready return also skipped the capability refresh, so a disconnect could leave the HDR capabilities frozen even before these changes. It stands alone at the tip, so it could be cherry-picked directly or folded in on a rebase, and I can drop it from this PR in that case. |
2 participants
aml_display_support_dv() caches its verdict in a function static the first time it reads dv_cap and never looks again. If the display changes later the verdict stays frozen until Kodi restarts. This is the same class of issue #73 fixed for the HDR capabilities, one layer down.
This moves the verdict into the same event-driven model. UpdateHDRCapabilities() already reads dv_cap on init and on hotplug, so it now derives the verdict there and pushes it into AMLUtils. The second commit removes the extra statics layered on top in the Dolby Vision wrappers so they follow the live verdict, and the third keeps the DV settings visibility in step from the same path. Three cached statics are replaced by one live flag, and the existing call signatures are untouched. A missing or unreadable dv_cap now counts as unsupported, so the verdict matches what the kernel reports instead of assuming support.
The reachable everyday case is boot order with an AVR. If the box powers up before the AVR or TV, it boots without a DV handshake, and until now that first impression was permanent for the whole session. The verdict stayed frozen on unsupported and all the DV settings stayed hidden until restart. With this change that boot has no lasting consequence. Once the chain comes up the hotplug re-reads dv_cap and DV engages on the next playback without a restart. The sink specific settings now show and hide with the live verdict, while the toggles other settings depend on stay visible on a DV capable box so a disabled option always has its reason on screen. A cable pull counts as losing the sink, so the verdict drops and those settings hide until a display returns. Setting values are only written at boot.