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

Merge the On-Screen-Controller #172

Closed
ChrisK2 opened this issue Aug 6, 2013 · 19 comments
Closed

Merge the On-Screen-Controller #172

ChrisK2 opened this issue Aug 6, 2013 · 19 comments

Comments

@ChrisK2
Copy link
Member

ChrisK2 commented Aug 6, 2013

http://puu.sh/3LRh0

@pigoz
Copy link
Member

pigoz commented Aug 6, 2013

Can you put the OSC on a separate repository in the mpv organization? People might actually start using it and give feedback/report bugs. More importantly, are all the C parts even merged?

@ChrisK2
Copy link
Member Author

ChrisK2 commented Aug 6, 2013

yeah, that's probably the best. And no, the lua implementation is still missing from master, but will hit it soon.

@ghost
Copy link

ghost commented Aug 8, 2013

I see at least three blockers:

  1. Whether track IDs should start at 0 or 1 (mpv and OSC currently disagree, which is bullshit)
  2. That the OSC appears on very minor mouse movement, which sometimes happen sporadically. This can lead to the OSC suddenly blocking the screen, even if you're not using the mouse at all. This is very annoying and I have experienced it myself.
  3. The progress "diamond" looks ugly compared to the progress bar that was used previously.

Some more issues that could be discussed or that are in the way:

  • I want to run Lua scripts in their separate threads. I haven't got to it yet, and I don't want to hold back everyone just for that, so I can move that to after merge. But this change could be disruptive.
  • The OSC requires Lua support, and that should be properly documented, and some things have to be improved before it's ready for general use.
  • We have to find out whether users like having the OSC on by default at all. (But this pretty much requires a merge and enabling it by default.)
  • The (somewhat inherent) input lag of the OSC, espcially mouse input, could be annoying.
  • I haven't added the ability to set options for Lua scripts. (Do we need that at all? The script could just load its own config file.)
  • Probably more.

@ChrisK2
Copy link
Member Author

ChrisK2 commented Aug 9, 2013

Edit: For the people who aren't on IRC frequently, we are talking about this: http://puu.sh/3LRh0

Whether track IDs should start at 0 or 1 (mpv and OSC currently disagree, which is bullshit)

The inconsistency is bullshit, I agree, but displaying 0-based counting to end-users is even more bullshit. Displaying "Track 0/1" will make no sense to someone who isn't familiar with programming. The OSC is completely consistent in using 1-based counting. Users would only experience inconsistency if they use the --xid option (which they don't have to, as they can just change tracks with the OSC). You are welcome to change mpv's internal track-IDs to 1-based-counting, which should probably be discussed in a separate issue. It should also be noted, that the OSC supports displaying the "raw" track IDs as-is as well.

That the OSC appears on very minor mouse movement, which sometimes happen sporadically. This can lead to the OSC suddenly blocking the screen, even if you're not using the mouse at all. This is very annoying and I have experienced it myself.

Okay, I still don't get were this comes from. The OSC is using the deadzone method for weeks now, so if you park your mouse somewhere at the top of the screen there is very major mouse movement needed to make the OSC show up.

The progress "diamond" looks ugly compared to the progress bar that was used previously.

This is your personal taste. I like the diamond. On top of that, a progress bar is not a GUI element users can normally interact with, while the "moving indicator" is often used in other player software as well. (It should also be noted that users who really don't like this can change the slider back to progress bar style in the script.)

We have to find out whether users like having the OSC on by default at all. (But this pretty much requires a merge and enabling it by default.)

The OSC is targeted at "average users" who may never read the manual and never find out it even exists. So it should be enabled by default. People who don't like it will manage to find the option to turn it off.

The (somewhat inherent) input lag of the OSC, espcially mouse input, could be annoying.

Very minor and not annoying at all in the weeks I've been using it now

I haven't added the ability to set options for Lua scripts. (Do we need that at all? The script could just load its own config file.)

Seeing how some people eagerly disagree with my design-choices, some configuration method would probably be welcome to save users from the effort of maintaining their modified copy of osc.lua somewhere. I'll look into using an own config file.

@ghost
Copy link

ghost commented Aug 9, 2013

The inconsistency is bullshit, I agree, but displaying 0-based counting to end-users is even more bullshit.

I'll leave this to the users to decide. Personally, I care more about the consistency than the actual start offset, but I'm also wary about changing the current default.

But how to we ask the users?

  1. That the OSC appears on very minor mouse movement, which sometimes happen sporadically. This can lead to the OSC suddenly blocking the screen, even if you're not using the mouse at all. This is very annoying and I have experienced it myself.

Okay, I still don't get were this comes from. The OSC is using the deadzone method for weeks now, so if you park your mouse somewhere at the top of the screen there is very major mouse movement needed to make the OSC show up.

I don't want to have to explicitly "park" my mouse somewhere just because the software is too stupid to handle this reasonably. I'm pretty sure 100% of our current users think the same way, because, you know, most run it from the terminal and use keyboard control. It seems you have plans about gathering new users, which is fine and awesome, but I don't want to alienate the current userbase.

The progress "diamond" looks ugly compared to the progress bar that was used previously.

This is your personal taste.

It's my project, so my taste takes priority. Anyway, I don't want you to "force" to change this, so once again it would be good to ask the users, and if they have strong opinions about it, that takes priority above all.

The (somewhat inherent) input lag of the OSC, espcially mouse input, could be annoying.

Very minor and not annoying at all in the weeks I've been using it now

I'm not sure about this myself; again user feedback would be nice. Also, some VOs might lag more, and future improvements to video playback might make the lag worse. (That's all because video playback is optimized for throughput and exact timing, not real-time user interaction.)

I'll look into using an own config file.

Do you need any mpv provided functions for this?

@pigoz
Copy link
Member

pigoz commented Aug 9, 2013

Seeing how some people eagerly disagree with my design-choices, some configuration method would probably
be welcome to save users from the effort of maintaining their modified copy of osc.lua somewhere. I'll look into
using an own config file.

IMHO this problem is greatly undervalued.. This is why I think we need a period of testing with the OSC outside the main tree. I'm actually a proponent of having a plugins project in the organization. Other plugins could be added to do for example scrobbling, IRC status updates, growl notifications and god knows what else. Does it all have to finish in the main tree? Probably not.

And have the plugins autoloaded if put somewhere like ~/.mpv/plugins/lua/autoload and use the existing config path functionality in mpv for plugins lookup.

This is similar to what weechat does for example. The added benefit is a packager could add some default plugins if he wants.

@ghost
Copy link

ghost commented Aug 9, 2013

On Fri, 09 Aug 2013 02:34:36 -0700
Stefano Pigozzi notifications@github.com wrote:

Seeing how some people eagerly disagree with my design-choices, some configuration method would probably
be welcome to save users from the effort of maintaining their modified copy of osc.lua somewhere. I'll look into
using an own config file.

IMHO this problem is greatly undervalued.. This is why I think we need a period of testing with the OSC outside the main tree. I'm actually a proponent of having a plugins project in the organization. Other plugins could be added to do for example scrobbling, IRC status updates, growl notifications and god knows what else. Does it all have to finish in the main tree? Probably not.

And have the plugins autoloaded if put somewhere like ~/.mpv/plugins/lua/autoload and use the existing config path functionality in mpv for plugins lookup.

Hm... could work.

This is similar to what weechat does for example. The added benefit is a packager could add some default plugins if he wants.

Should be no problem to test this while the OSC is not enabled by
default (making it very easy to let users enable it for testing). Also,
now that we have releases it should be ok to have not-quite-polished
work in git master.

The OSC is probably also a bit different from other possible plugins
because it significantly enhances the user interface.

@ChrisK2
Copy link
Member Author

ChrisK2 commented Aug 10, 2013

Okay, let me start with highlighting a very important but apparently ignored part of my previous post:

The OSC is targeted at "average users".

More specifically, users new to mpv or coming from "full-GUI-OSes", who want to be able to use the essential functions without memorizing a page of shortcuts and digging through the endless manuals first. It is not aimed at people who "run it from the terminal and use keyboard control" since, why would they want to use something like this anyway?
And yes, mpv has a lot of potential beyond the current user-base, which will be only acquirable with some sort of GUI. So my ultimate goal is to open up mpv to more general users, mainly OS X and Windows users. This brings me to the next point, your concern about alienating the current user-base. I'm apparently not very familiar with it, but you make it sound like any piece of GUI shoved in their face might cause them a seizure or to burn your house down. However, I'm confident that this won't happen since ... what – besides putting "osc=no" in their config – are they supposed to be doing? Go back to mplayer2?

So yes, "asking" the users is obviously the best way for feedback. But the only way to do this on a large scale is to just merge it in master, enable it by default and wait for the complaints to roll in.

Last but not least, @pigoz's proposal of a plugin architecture sounds very favorable and should be persuaded further.

Do you need any mpv provided functions for this?

I probably need a way to query the path to ~/.mpv

@ghost
Copy link

ghost commented Aug 14, 2013

I remembered another issue: for some reason, the OSD symbols (with classic OSD) are smaller with the modified OSD font.

@ChrisK2
Copy link
Member Author

ChrisK2 commented Aug 14, 2013

I have no idea why this happens, I don't think I changed anything about the old glyphs or the font-metrics in general, but I'll investigate it.

@nagisa
Copy link

nagisa commented Aug 21, 2013

Just a question: How gnome-mplayer/smplayer way is wrong/bad to add a GUI to mplayer/mpv?

A~nd another question: What would happen in case I play some (music) file which currently doesn't show a mpv window?

@ghost
Copy link

ghost commented Aug 21, 2013

Just a question: How gnome-mplayer/smplayer way is wrong/bad to add a GUI to mplayer/mpv?

It's not wrong, but the old slave protocol sucks. I plan to add a client library to allow external GUIs to control/embed mpv, though, see issue #97.

What would happen in case I play some (music) file which currently doesn't show a mpv window?

Nothing. It would behave as with OSC disabled. We could maybe add a mode where it creates an empty window or so if there is no video track.

@ghost
Copy link

ghost commented Aug 24, 2013

Another issue: the OSC needs Lua. We are fine with any Lua (5.1, 5.2, luajit), but whatever we use can clash with the Lua version used by libquvi. Not sure if we can play any linker acrobatics to work this around, but for now I'm clueless how to solve this.

@ChrisK2
Copy link
Member Author

ChrisK2 commented Aug 27, 2013

for some reason, the OSD symbols (with classic OSD) are smaller with the modified OSD font.

Apparently, the OSD-symbol-font had somewhat broken metrics before, causing the symbols to be rendered too large. So far this was simply compensated by applying a \fs-5 tag to them (see

// NOTE: \fs-5 to reduce the size of the symbols in relation to normal text.
). Now that the font has been run through a proper editor, it seems the metrics got fixed along the way making the symbols render way to small due to the \fs-5 tag. Changing it to \fs-1 fixed this. (153ac1c)

@ghost
Copy link

ghost commented Aug 27, 2013

Now that the font has been run through a proper editor, it seems the metrics got fixed along the way

A "proper" editor shouldn't change any metrics. Also, the font didn't change when it was converted from pfb to otf.

@giselher
Copy link
Member

Another issue: the OSC needs Lua. We are fine with any Lua (5.1, 5.2, luajit), but whatever we use can clash with the Lua version used by libquvi. Not sure if we can play any linker acrobatics to work this around, but for now I'm clueless how to solve this.

Maybe the pkg-config check for lua should be above the others (lua5.2, lua5.1), because I guess this one would be the one check preferred by the distribution.

@ghost
Copy link

ghost commented Aug 27, 2013

Since Lua has no official pkg-config files, this is highly distro specific. For example, on Debian, only "lua5.2" and "lua.5.1" exist, while others systems have "lua" only. For now, it looks like you can't really win.

@giselher
Copy link
Member

I guess this issue can be closed now?

@ghost
Copy link

ghost commented Sep 26, 2013

Yep.

@ghost ghost closed this as completed Sep 26, 2013
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

4 participants