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.

Sign up for GitHub

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

Jump to bottom

Support MPRIS loop and volume change #749

Merged
merged 8 commits into from
Aug 21, 2022
+102 −21
Merged

Support MPRIS loop and volume change #749

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6e1c50e
Support MPRIS loop
foonathan Jun 14, 2022
bba4990
Style changes for review
foonathan Jun 14, 2022
9932fd7
Fix mpris playback status on play/pause
foonathan Jun 22, 2022
e7e8738
Use triple equals
foonathan Jun 26, 2022
2499f57
Use triple equals in more places
foonathan Jun 26, 2022
d9c5106
Add MPRIS volume control
foonathan Jul 11, 2022
dfba3d9
Support precise volume changes in MPRIS when possible
foonathan Jul 11, 2022
0f96da9
Change volume observer
foonathan Jul 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions plugins/shortcuts/mpris.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,35 @@ function registerMPRIS(win) {
let currentSeconds = 0;
ipcMain.on('timeChanged', (_, t) => currentSeconds = t);

let currentLoopStatus = undefined;
let manuallySwitchingStatus = false;
ipcMain.on("repeatChanged", (_, mode) => {
if (manuallySwitchingStatus)
return;

if (mode == "Repeat off")
currentLoopStatus = "None";
else if (mode == "Repeat one")
currentLoopStatus = "Track";
else if (mode == "Repeat all")
currentLoopStatus = "Playlist";

player.loopStatus = currentLoopStatus;
});
player.on("loopStatus", (status) => {
// switchRepeat cycles between states in that order
const switches = ["None", "Playlist", "Track"];
const curIdx = switches.indexOf(currentLoopStatus);
const newIdx = switches.indexOf(status);

// Get a delta in the range [0,2]
const delta = ((newIdx - curIdx) % 3 + 3) % 3;
Copy link
Collaborator

@Araxeus Araxeus Jun 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of the first modulo? (newIdx - curIdx) can only be in the range (-2, 2) then adding 3 is the exact result we need

Can be simplified to just

const delta = (newIdx - curIdx + 3) % 3;
See calc

0 = (0 - 0) % 3 + 3) % 3
0 = (0 - 0 + 3) % 3

2 = (0 - 1) % 3 + 3) % 3
2 = (0 - 1 + 3) % 3

1 = (0 - 2) % 3 + 3) % 3
1 = (0 - 2 + 3) % 3

1 = (1 - 0) % 3 + 3) % 3
1 = (1 - 0 + 3) % 3

0 = (1 - 1) % 3 + 3) % 3
0 = (1 - 1 + 3) % 3

2 = (1 - 2) % 3 + 3) % 3
2 = (1 - 2 + 3) % 3

2 = (2 - 0) % 3 + 3) % 3
2 = (2 - 0 + 3) % 3

1 = (2 - 1) % 3 + 3) % 3
1 = (2 - 1 + 3) % 3

0 = (2 - 2) % 3 + 3) % 3
0 = (2 - 2 + 3) % 3

for (let a = 0; a <= 2; a++) {
    for (let b = 0; b <= 2; b++) {
        console.log(`${((a - b) % 3 + 3) % 3} = (${a} - ${b}) % 3 + 3) % 3`);
        console.log(`${(a - b + 3) % 3} = (${a} - ${b} + 3) % 3`);
    }
}

also, I personally think that short variable names aren't helpful, its better if they are clear and informative
could maybe be named currentIndex and targetIndex instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of the first modulo?

I originally had just (newIdx - curIdx) % 3, but that has the wrong sign. I then simply added the general code. Good catch.


manuallySwitchingStatus = true;
songControls.switchRepeat(delta);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is a bit of a hack (it presses the button the appropriate number of times to go to the desired state), but I can't see a better way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried making it work using the youtube api, but its weird and doesn't visually change the button state

So your solution is probably the best bet

manuallySwitchingStatus = false;
})

player.getPosition = () => secToMicro(currentSeconds)

player.on("raise", () => {
Expand Down
5 changes: 4 additions & 1 deletion providers/song-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ module.exports = (win) => {
go1sBack: () => pressKey(win, "h", ["shift"]),
go1sForward: () => pressKey(win, "l", ["shift"]),
shuffle: () => pressKey(win, "s"),
switchRepeat: () => pressKey(win, "r"),
switchRepeat: (n = 1) => {
for (let i = 0; i != n; ++i)
Copy link
Collaborator

@Araxeus Araxeus Jun 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++i is useless here since the value isn't used
i++ is more appropriate in this scenario (tho ultimately it's the same, the syntax is more clear)

also instead of i != n you should use i < n to avoid edge cases (this is standard)

Copy link
Contributor Author

@foonathan foonathan Jun 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny. I come from a C++ background, and if someone wrote for (auto i = 0; i < n; i++) I would have requested it changed to the way I wrote it, as that's the standard there :D

pressKey(win, "r");
},
// General
volumeMinus10: () => pressKey(win, "-"),
volumePlus10: () => pressKey(win, "="),
Expand Down
11 changes: 11 additions & 0 deletions providers/song-info-front.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ module.exports = () => {
if (config.plugins.isEnabled('tuna-obs') ||
(is.linux() && config.plugins.isEnabled('shortcuts'))) {
setupTimeChangeListener();
setupRepeatChangeListener();
}
const video = $('video');
// name = "dataloaded" and abit later "dataupdated"
Expand Down Expand Up @@ -63,3 +64,13 @@ function setupTimeChangeListener() {
});
progressObserver.observe($('#progress-bar'), { attributeFilter: ["value"] })
}

function setupRepeatChangeListener() {
const repeatObserver = new MutationObserver(mutations => {
ipcRenderer.send('repeatChanged', mutations[0].target.title);
});
repeatObserver.observe($('#right-controls .repeat'), { attributeFilter: ["title"] });

// Emit the initial value as well; as it's persistent between launches.
ipcRenderer.send('repeatChanged', $('#right-controls .repeat').title);
}