Open stereomato opened 1 month ago
Dudemanguy
commented
1 month ago I'm not able to reproduce on git master or 0.38 on mutter 46.3.1. Flickering when subtitles show sounds like a compositor and/or driver bug to me. We just draw shm buffers for the subtitles/osd and attach it to a different surface and hand that off to the compositor. Could you try a different compositor (weston, kwin, whatever) and see if the same thing happens?
mahkoh
commented
1 month ago GNOME is likely switching between direct scanout and compositing whenever subtitles appear disappear. Alternatively GNOME might be using an overlay plane for the subtitles and that plane might be getting activated/deactivated.
stereomato
commented
1 month ago I could reproduce it with weston. Exactly how it happens with Gnome. Also, this only happens with dmabuf-wayland, not with gpu-next. Maybe Intel specific, then?
llyyr
commented
1 month ago can reproduce on wlroots with amdgpu as well. only happens when direct scanout is active
Dudemanguy
commented
1 month ago Well I'm not a compositor developer but I assumed this was supposed to work with direct scanout without any weirdness? In fact, rmader himself was the one that submitted the patch in #12237 to make it so direct scanout was possible on mutter when the the osd was not being shown. I would assume that, at least at the time, nothing was flickering when the osd would hide/appear. @rmader: any insight here?
rmader
commented
1 month ago GNOME is likely switching between direct scanout and compositing whenever subtitles appear disappear.
This is most likely the case and the slight color mismatch is unfortunately expected for the time being until we have the color-management and color-representation protocols. Until then compositors have to guess and unfortunately Mutter and Weston currently default to e.g. bt601 color curves - which is slightly different from bt709.
While we could tweak that - e.g. defaulting to bt709 as much more content uses that - most work in that area just focuses on getting the proper protocols landed/implemented. On the side of Gnome we'll probably be too late for 47, however 48 will likely work out.
rmader
commented
1 month ago P.S.: as an intermediate solution it might make sense to always keep the overlay visible in the given situation. That will increase power consumption accordingly of course.
Dudemanguy
commented
1 month ago Hmm I see. So in that case we can just pretend the osd surface always has contents if the colorspace isn't bt601. Does this work for you guys?
diff --git a/video/out/vo_dmabuf_wayland.c b/video/out/vo_dmabuf_wayland.c
index e2dafdaa6f..6322a02cee 100644
--- a/video/out/vo_dmabuf_wayland.c
+++ b/video/out/vo_dmabuf_wayland.c
@@ -541,7 +541,7 @@ static void resize(struct vo *vo)
set_viewport_source(vo, src);
}
-static bool draw_osd(struct vo *vo, struct mp_image *cur, double pts)
+static bool draw_osd(struct vo *vo, struct mp_image *cur, enum pl_color_system sys, double pts)
{
struct priv *p = vo->priv;
struct mp_osd_res *res = &p->screen_osd_res;
@@ -562,7 +562,9 @@ static bool draw_osd(struct vo *vo, struct mp_image *cur, double pts)
MP_ARRAY_SIZE(act_rc), &num_act_rc,
mod_rc, MP_ARRAY_SIZE(mod_rc), &num_mod_rc);
- p->osd_surface_has_contents = num_act_rc > 0;
+ // Unfortunately since there is no way to signal the actual colorspace to
+ // compositors, we have to pretend this is always true if it is not bt601.
+ p->osd_surface_has_contents = sys == PL_COLOR_SYSTEM_BT_601 ? num_act_rc > 0 : true;
if (!osd || !num_mod_rc)
goto done;
@@ -591,6 +593,7 @@ static void draw_frame(struct vo *vo, struct vo_frame *frame)
struct vo_wayland_state *wl = vo->wl;
struct buffer *buf;
struct osd_buffer *osd_buf;
+ enum pl_color_system sys = PL_COLOR_SYSTEM_UNKNOWN;
double pts;
if (!vo_wayland_check_visible(vo)) {
@@ -614,16 +617,16 @@ static void draw_frame(struct vo *vo, struct vo_frame *frame)
if (buf && buf->frame) {
struct mp_image *image = buf->frame->current;
+ sys = image->params.repr.sys;
wl_surface_attach(wl->video_surface, buf->buffer, 0, 0);
wl_surface_damage_buffer(wl->video_surface, 0, 0, image->w,
image->h);
-
}
}
osd_buf = osd_buffer_get(vo);
if (osd_buf && osd_buf->buffer) {
- if (draw_osd(vo, &osd_buf->image, pts) && p->osd_surface_has_contents) {
+ if (draw_osd(vo, &osd_buf->image, sys, pts) && p->osd_surface_has_contents) {
wl_surface_attach(wl->osd_surface, osd_buf->buffer, 0, 0);
wl_surface_damage_buffer(wl->osd_surface, 0, 0, osd_buf->image.w,
osd_buf->image.h);
It might be worth vendoring in the color management protocol on our side and using it instead of waiting for it to be official.
rmader
commented
1 month ago So in that case we can just pretend the osd surface always has contents if the colorspace isn't bt601.
Yeah, should somewhat work for the time being.
It might be worth vendoring in the color management protocol on our side and using it instead of waiting for it to be official.
That's kinda happening already, however the issue is rather the compositor implementations, not the protocol. At least for Weston and Mutter I can attest though that we're much closer now to have thing working.
If you're interested, here's a small Gstreamer prototype that I'll be further working on next month: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/6830
Dudemanguy
commented
1 month ago For this specific problem, it seems like color-representation would suffice. I would have no problem vendoring that for mpv usage since it's pretty important for our use case and if it would be helpful for compositors.
Of course, color-management is of interest to us as well but that seems quite a bit more complicated.
mpv Information
Other Information
Reproduction Steps
Expected Behavior
Colors not changing/flickering like that, regardless of fullscreen or windowed mode.
Actual Behavior
Colors change/flicker strangely, when in fullscreen mode, and subtitles display or the osd is drawn.
Log File
output.txt
Sample Files
No response
I carefully read all instruction and confirm that I did the following:
--log-file=output.txt.