search

woodruffw / ff2mpv

A Firefox/Chrome add-on for playing URLs in mpv.
https://addons.mozilla.org/en-US/firefox/addon/ff2mpv/
Other
521 stars 52 forks source link

Feat/mpv profiles #108

Closed DanSM-5 closed 10 months ago

DanSM-5 commented 11 months ago

Feature: mpv profiles

Add a set of command line arguments into its own profile to be called in a new entry in the context menu of ff2mpv.

Issue

I've had the need to call mpv with different command line arguments due to the different resolutions of my monitors and to avoid some issues with hls in youtube https://github.com/yt-dlp/yt-dlp/issues/7824 (I can't open the issue anymore but it is listed here https://github.com/yt-dlp/yt-dlp/issues/3766).

Motivation

I found having to change mpv.conf or ff2mpv.py every time I needed to change some command line arguments a bit annoying. While using mpv directly from the command line is fine, I wanted to have the convenience of ff2mpv's right click play. I think I found a good solution and I think other users my benefit from this.

Solution

Adding new entries in the context menu for mpv per each profile created by the user. In order to create a profile you need to add it manually in the addon/extension options. A profile is simply a name and a set of command line arguments. If the given profile is clicked, it will send the options to the native client which just forward the arguments to the mpv command. Current behavior is preserved with the default profile "Play in MPV" which doesn't add any additional argument.

Demo

https://drive.google.com/file/d/1WOV7gpezys_B4Ox43H4lChlM0mbdjim3/view?usp=share_link

DanSM-5 commented 11 months ago

@woodruffw Let me know what you think.

woodruffw commented 11 months ago

Thanks @DanSM-5! I really like the demo.

I'll give the code itself a review tomorrow.

DanSM-5 commented 11 months ago

I realized I forgot to update the ruby client. I added what looks correct based on some research I did but I'd appreciate if you could validate the functionality.

DanSM-5 commented 11 months ago

I found an issue. In summary, the profile.content array that I was trying to leave in the closure was causing a reference to dead object. I found that it happened because the profile object is disposed at the moment you close the options menu of the extension.

I did that on purpose to avoid an extra lookup in the storage but that seems like the best alternative to avoid the issue. I've pushed the commit.

Image for reference of the dead object: image

DanSM-5 commented 11 months ago

@woodruffw did you get a chance to review the PR?

woodruffw commented 11 months ago

Not yet, sorry. It's on my list of things to do tomorrow.

woodruffw commented 10 months ago

Merged. Let me know if you'd like a release here, otherwise I'll probably do it sometime next week.

DanSM-5 commented 10 months ago

@woodruffw feel free to do the release next week.

woodruffw commented 8 months ago

I completely forgot to release this. I'll try and remember to do this tonight...

DanSM-5 commented 8 months ago

No worries. Originally I wanted the feature before going out in a trip but I ended up with almost no time to watch youtube, so it didn't affect me.