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

Submission: cycle_video_rotate.lua #3869

Closed
ghost opened this issue Dec 4, 2016 · 9 comments
Closed

Submission: cycle_video_rotate.lua #3869

ghost opened this issue Dec 4, 2016 · 9 comments

Comments

@ghost
Copy link

ghost commented Dec 4, 2016

Edit: The final version of the script has been published on the wiki, along with a few other scripts:

https://github.com/mpv-player/mpv/wiki/User-Scripts

  • auto-keep-gui-open
    Intelligently switches mpv's "keep-open" behavior based on whether you are running in video-mode or audio-only mode.

  • cycle-video-rotate
    Allows you to perform video rotation which perfectly cycles through all 360 degrees without any glitches.

  • multi-command-if
    Very powerful conditional logic and multiple action engine for your keybindings, without having to write a single line of code!

  • quick-scale
    Quickly scale the video player to a target size, with full control over target scale and max scale. Helps you effortlessly resize a video to fit on your desktop, or any other video dimensions you need!


Old post:

I noticed that you have an internal repository of scripts. I also noticed that you've had questions from users about how to rotate a video.

Method 1: Add/Subtract

Alt+LEFT add video-rotate -90
Alt+RIGHT add video-rotate 90

Result: 0..90..180..270..359 (this is where all hell breaks loose; the video is now unevenly rotated).
Verdict: Garbage method.

Method 2: Cycle a set of pre-determined values.

Alt+LEFT cycle-values "!reverse" video-rotate "90" "180" "270" "0"
Alt+RIGHT cycle-values video-rotate "90" "180" "270" "0"

Result: Always even rotation. But if your first action on a video is to rotate left, your next rotation will be "0" (the default rotation for a video), so you actually have to press left twice to get it to rotate left.
Verdict: Almost good... so close, but not good enough for a project the quality of mpv.

Method 3: My "Cycle_Video_Rotate" script.

~/.config/mpv/scripts/cycle_video_rotate.lua: (Edit: See updated version of this script below)

function cycle_video_rotate(direction)
	-- Calculate what the next rotation value should be.
	newrotate = mp.get_property_number("video-rotate")
	if direction == "clockwise" then
		newrotate = newrotate + 90
	else
		newrotate = newrotate - 90
	end
	-- Wrap value to correct range (0 (aka 360) to 359).
	if newrotate >= 360 then
		newrotate = newrotate - 360
	elseif newrotate < 0 then
		newrotate = newrotate + 360
	end
	-- Change rotation and tell the user.
	mp.set_property_number("video-rotate", newrotate)
	mp.osd_message("Rotate: " .. newrotate)
end

mp.register_script_message("Cycle_Video_Rotate", cycle_video_rotate)

~/.config/mpv/input.conf

# Rotate video in 90 degree increments.
Alt+LEFT script-message Cycle_Video_Rotate "counter-clockwise"
Alt+RIGHT script-message Cycle_Video_Rotate "clockwise"

Result: Works instantly in any direction without needing multiple keypresses. Always aware of and basing itself on the current rotation value, thus never "jumping" to an unexpected rotation. Always uses even 90-degree increments as long as the users themselves haven't applied any uneven rotation before calling this function (in that case, it fully preserves their offset and does the 90 degree rotation on top of that).
Verdict: Get this included into mpv! ;-) It's so much slicker.

Take care!

@ghost
Copy link

ghost commented Dec 4, 2016

You have a point, but we usually don't add scripts to the repo anymore.

Also I think the reverse mode of property cycling should probably be improved.

@kkkrackpot
Copy link
Contributor

kkkrackpot commented Dec 4, 2016

@SteveJobzniak maybe https://github.com/mpv-player/mpv/wiki/User-Scripts ?

Also maybe something like:

# direction is 1 or -1
signed int direction, newrotate
newrotate=newrotate+90*direction
if (newrotate<0 OR newrotate=>360) then newrotate=newrotate-360*direction

Didn't test it myself, sorry...

@haasn
Copy link
Member

haasn commented Dec 4, 2016

Personally I would find it more intuitive and less prone to edge cases like these if the “cycle” logic simply worked by finding the current value in the list and picking the next one, defaulting to the first element if the current value was not in the list.

That's how I implemented :set --cycle in qutebrowser and I have not run into any surprising behavior there.

@ghost
Copy link
Author

ghost commented Dec 4, 2016

@haasn But that would break existing code and bindings that rely on cycle going from left to right, that don't expect the cycle to guess that we're somewhere else.

@fhlfibh There's no such thing as a "signed" or "int" in Lua, but yeah I could definitely use tricks like that to shorten the code, at the expense of readability. I haven't coded Lua in about 6 years so I struggled a bit to remember how the language works and definitely missed a few tricks. I'm more of a C guy. Anyway, the tricks you propose are definitely something I'd prefer in C. But this is Lua and is meant to be read by mere mortals (end-users) who may want to tweak the code for their needs, so I'll avoid adding those unreadable tricks to this script.

@wm4 Perhaps cycle could have an optional "assume we're currently at index X" initialization parameter.

Alt+LEFT cycle-values "!reverse" "index0" video-rotate "0" "90" "180" "270"
Alt+RIGHT cycle-values "index0" video-rotate "0" "90" "180" "270"

That would initialize the cycle as if we're already at index 0 (the first entry). So cycling forward would go to "90". Cycling backward would go to "270". And without that parameter, it would assume we aren't anywhere in the cycling list yet (that's its current behavior) and would pick "0" if going right or "270" if going left.

Adding the feature this way wouldn't break any old scripts.

Do you think there would be a general value to adding such an "assume we're at index X" feature to cycling, beyond the fact that it would allow mpv to natively do video rotation cycling more nicely?

@ghost
Copy link

ghost commented Dec 4, 2016

@haasn there's currently no way to test for equality of 2 values. We could convert the current value to a string and then compare them, but that would work badly for floats.

@ghost
Copy link
Author

ghost commented Dec 4, 2016

Updated version of ~/.config/mpv/scripts/cycle_video_rotate.lua to reduce code repetition:

function cycle_video_rotate(direction)
	-- Calculate what the next rotation value should be.
	newrotate = mp.get_property_number("video-rotate")
	newrotate = newrotate + (direction == "clockwise" and 90 or -90)
	-- Wrap value to correct range (0 (aka 360) to 359).
	if (newrotate < 0 or newrotate >= 360) then
		newrotate = newrotate + (newrotate < 0 and 360 or -360)
	end
	-- Change rotation and tell the user.
	mp.set_property_number("video-rotate", newrotate)
	mp.osd_message("Rotate: " .. newrotate)
end

-- Bind this via input.conf, ie. 'Alt+r script-message Cycle_Video_Rotate "clockwise"':
mp.register_script_message("Cycle_Video_Rotate", cycle_video_rotate)

Feel free to either bundle this, or improve "cycle". Not sure what's best to be honest. I haven't seen any other cycle-able values that would benefit from being able to set the start-index, so it may be a shame to add the "indexX" start-index feature just for video-rotate.

@haasn
Copy link
Member

haasn commented Dec 4, 2016

Incidentally, this is how I rotate with my image.lua:

-- Rotates the video while maintaining 0 <= prop < 360
function rotate_video(amt)
    local rot = mp.get_property_number("video-rotate")
    rot = (rot + amt) % 360
    mp.set_property_number("video-rotate", rot)
end

mp.add_key_binding(nil, "rotate-video", rotate_video)

and in input.conf

CTRL+RIGHT script-message rotate-video 90
CTRL+LEFT script-message rotate-video -90
CTRL+DOWN no-osd set video-rotate 0

If you want native integration for this, making the rotation properties use modular arithmetic instead of hard-capping the value would be the best approach (i.e. setting the property to 365 would end up actually setting it to 5). I don't think extending cycle-values is really necessary for any of the other use cases.

@ghost
Copy link
Author

ghost commented Dec 5, 2016

@haasn That's even better, with its ability to do uneven rotation if the user wants that. Great work.

Well, there are now plenty of ideas how mpv could have better built-in rotation out of the box:

  1. Add an "indexX" property to cycle which tells it which index to assume we're starting at, to avoid the need to rotate twice to get it to rotate past the value we're already at. This is the worst solution of all, though, because it's still unaware of the video's actual current rotation value.
  2. Bundle a script that's some kind of cleaned up and merged version of haasn's and my code, with some nice function name.
  3. Or create a new built-in function which basically does the same thing as the scripts: Letting the user rotate left/right by a certain amount, and wrapping the rotation value properly when needed.

Either way, users surely would appreciate a fixed/improved built-in way to rotate videos effortlessly in mpv. Easy rotation is kind of a basic feature, and I've seen people ask about it and always end with some disappointment when they see the limitations of all of the current non-script methods. :-)

Edit: Here's the final script version which employs the ideas from haasn:

function cycle_video_rotate(amt)
	-- Ensure that amount is a base 10 integer.
	amt = tonumber(amt, 10)
	if amt == nil then
		mp.osd_message("Rotate: Invalid rotation amount")
		return nil -- abort
	end
	-- Calculate what the next rotation value should be,
	-- and wrap value to correct range (0 (aka 360) to 359).
	local newrotate = mp.get_property_number("video-rotate")
	newrotate = ( newrotate + amt ) % 360
	-- Change rotation and tell the user.
	mp.set_property_number("video-rotate", newrotate)
	mp.osd_message("Rotate: " .. newrotate)
end

-- Bind this via input.conf. Example:
--   Alt+LEFT script-message Cycle_Video_Rotate -90
--   Alt+RIGHT script-message Cycle_Video_Rotate 90
mp.register_script_message("Cycle_Video_Rotate", cycle_video_rotate)

@ghost
Copy link
Author

ghost commented Dec 17, 2016

I've now published this and a few other scripts on the wiki:

https://github.com/mpv-player/mpv/wiki/User-Scripts

  • auto-keep-gui-open
    Intelligently switches mpv's "keep-open" behavior based on whether you are running in video-mode or audio-only mode.

  • cycle-video-rotate
    Allows you to perform video rotation which perfectly cycles through all 360 degrees without any glitches.

  • multi-command-if
    Very powerful conditional logic and multiple action engine for your keybindings, without having to write a single line of code!

  • quick-scale
    Quickly scale the video player to a target size, with full control over target scale and max scale. Helps you effortlessly resize a video to fit on your desktop, or any other video dimensions you need!

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