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

x11 video out: a child window is created even if the wid option is given #255

Closed
maniacgit opened this issue Sep 22, 2013 · 18 comments
Closed

Comments

@maniacgit
Copy link

If you are using the --wid parameter to pass an existing x11 window for playback the x11 video out creates another child window with window from the --wid parameter as parent.

The expect behaviour is to use the window delegated through the wid parameter and not another child window. In mplayer and mplayer2 the wid parameter was handled this way.

The creation of the child window breaks the possibility to have another window from another application on the top. In the vdr play plugin this is used to render the vdr osd over the mpv/mplayer output.

@ghost
Copy link

ghost commented Sep 22, 2013

This is intentional. It's much more robust.

The creation of the child window breaks the possibility to have another window from another application on the top.

This should still work?

In the vdr play plugin this is used to render the vdr osd over the mpv/mplayer output.

This doesn't work with some video outputs (like everything which uses overlay), or is questionable at best (like with opengl).

It might be possible to let mpv render a user-defined overlay image though, if this goes into a helpful direction?

@ghost
Copy link

ghost commented Sep 22, 2013

PS: other ways might be:

  • vdr could find the mpv window, and draw into it
  • create another window on top of the mpv window; this should obscure the mpv window (but no alpha, except maybe with a compositing WM)

What does this cdr thing actually need? What does it draw?

@maniacgit
Copy link
Author

A composting WM should be avoided. Many vdr users don't even use a WM for their vdr installation (the vdr is directly connected to the tv and controlled by a remote control).

The vdr play plugin already creates an own window on top of the window which is submitted via the wid parameter to mpv. Since the play plugin is used by people who use a vdpau capable grahpics card the colorkey mechanism of vdpau is used to make the osd window transparent.

I've just recognized that the mpv x11 code, which creates the other child window, also sets this window to foreground. Maybe only this breaks the osd display since the new window is set over the osd window. I'll test this, if this fixes the issue would it be possible to skip this if a wid is given?

The possibility to render a user-defined overlay image would be the best option, it would be perfect if an alpha channel is also supported. But keep in mind that this needs the ability to be updated often (e.g. for progress display or navigating in the menu).

@maniacgit
Copy link
Author

I just gave it a try and didn't set the window from mpv to foreground but it's still on top and covers the osd. I think this happens because the window from mpv is created last. The only thing that works so far is preventing mpv from creating a child window (like mplayer/mplayer2). This is an option for my own environment but not for the normal user.

@ghost
Copy link

ghost commented Sep 24, 2013

Well, again, couldn't vdr just search for the mpv window, and draw into it directly?

But I'd even in mplayer/mplayer2 this worked pretty much only by chance, so it would be worth finding a proper solution, such as a RGBA overlay supported natively by the player.

What exactly does vdr want to render? Let's assume it absolutely needs RGBA bitmaps for now...

MPlayer can display overlays in grayscale (blended with a single color) using the overlay_add command. It expects a path to a pnm file. We could add something similar, but maybe sourcing a RGBA overlay from shared memory would be better. Maybe something like this:

overlay_add <id> <fd> <offset> <w> <h> <stride> <format> <x> <y> <dw> <dh>
overlay_remove <id>

id: user chosen id for overlay_remove
fd: UNIX file descriptor that should be mmap'ed to get the bitmap data
offset: offset for mmap
w, h: dimension of the bitmap in pixels
stride: size of a bitmap line (will typically be w*4)
format: 'ARGB' (to allow other image formats, should the need arise)
x, y: window coordinate of top/left corner where the bitmap should be displayed
dw, dh: display width of the overlay bitmap (the OSD bitmap can be scaled)

mpv would read the shared memory area every time the OSD needs to be redrawn. To prevent tearing, the application should keep a ringbuffer of 2 mapped buffers, and use a different one for each overlay_add. This design should be pretty efficient: in the best case (like with vdpau), mpv itself will never even read the bitmap data, but just pass a pointer to the shared memory to the graphics API.

Or do you think this would be overkill?

In any case, if think the overlay_add command as I proposed here is useful for you, I can implement it.

@maniacgit
Copy link
Author

The overlay_add command with the shared memory area would be a perfect solution for displaying the osd. If it's possible please implement it.

@ghost
Copy link

ghost commented Sep 25, 2013

OK, I can do that.

Just some more questions about issues which might complicate or simplify the implementation of this:

  • is there a need for multiple overlays?
  • if yes, should overlays be able to overlay other overlays?
  • is there a need at all to support scaling of overlay (w vs. dw)?
  • if yes, should overlays be able to have different scale factors?
  • should overlays work on screen or video coordinates? This is the most complicated issue, because some VOs always do OSD in video coordinates, while others use screen coordinates.

@maniacgit
Copy link
Author

is there a need for multiple overlays?
no

is there a need at all to support scaling of overlay (w vs. dw)?
no

should overlays work on screen or video coordinates? This is the most complicated issue, because some VOs always do OSD in video coordinates, while others use screen coordinates.
It should work on screen coordinates

@ghost
Copy link

ghost commented Sep 30, 2013

Pushed a test implementation to the branch named overlay (https://github.com/mpv-player/mpv/tree/overlay).

Is this useful for you?

I'm still undecided about some details.

@maniacgit
Copy link
Author

Thank you for your implementarion. I've just finished the integration and it works like a charm.

This solution is much better than the old one we've used with the vdpau colorkey.

@ghost
Copy link

ghost commented Oct 3, 2013

That's nice to hear! I'll overthink the overlay_add command and then merge it. (I won't change the fundamental way it works, but maybe the argument it takes.)

@ghost
Copy link

ghost commented Oct 3, 2013

By the way, where is the source code for your project located and where can I see how you use the overlay_add command?

@maniacgit
Copy link
Author

Currently in my private git because I'm not the maintainer of this project. But I already have contact to him about this matter and will provide patches to include this upstream.

But I can push a fork to github with my changes if you wish.

@ghost
Copy link

ghost commented Oct 3, 2013

That would be helpful. I don't want to second-guess how you used this feature.

@maniacgit
Copy link
Author

I've cleaned up my sources and pushed them to github. The commit which adds the feature is reachable under maniacgit/vdr-play@e28370f

@ghost
Copy link

ghost commented Oct 3, 2013

Hm ok, looked at some of the commits. I'm not sure whether creating and writing a file on each redraw could be a performance issue. Also, the file would of course have to be created in a secure way. It can be a bit hard to create a filename securely and pass it to another program because of asssociated security issues, which why I made overlay_add also accept FDs (so mkstemp() could be used). The FDs can be implicitly passed to mpv as part of the exec call.

Slave mode is currently in a sad state (and actually always was, even in mplayer), we will change it at some point, see issues #252 and #97.

Currently, you can use --status-msg to return time-pos and other properties, instead of having to query them on every frame or so.

The fps property is not always reliable. I'm not really sure how reliable it is at all; it depends on random things, like media file headers or ffmpeg implementation details.

Anyway, I'll oush overlay_add in the current or slightly modified form tomorrow.

ghost pushed a commit that referenced this issue Oct 5, 2013
Requested by github issue #255.

Does not work where mmap is not available (i.e. Windows).
@ghost
Copy link

ghost commented Oct 5, 2013

Pushed that branch with slight modifications. I think nothing about the command's parameters and semantics changed.

If anyone reading this ticket is wondering: mpv is still creating its own window even with --wid; all I did here was adding a command to allow user-defined overlays.

@ghost ghost closed this as completed Oct 5, 2013
@divVerent
Copy link
Member

BTW, just noticed something else: the child window approach can still be used with --wid.

The difference is that the calling application needs a different window structure.

What works with mplayer:

  • WM Border
    • Application
      • Player Window (--wid)
        • OSD Window of Application

What still works despite mpv's subwindow:

  • WM Border
    • Application
      • Player Container
        • Player Window (--wid)
          • mpv's Subwindow
        • OSD Window of Application

The reason is that mpv raises the window it created, however it can only do so among its siblings. The introduction of this intermediate window makes mpv's Subwindow have no siblings, fixing the issue.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants