Node-MPV icon indicating copy to clipboard operation
Node-MPV copied to clipboard

percent-pos property is 'null' before quitting

Open vankasteelj opened this issue 4 years ago • 8 comments

Bug Description

When listening to 'statuschange' event and adding a 'percent-pos' property to observe, when quitting MPV, the percent-pos is set to 'null' on MPV > 0.29

How To Reproduce

let position = 0;

mpv.start().then(() => mpv.load(<myfile>)).then( () => mpv.observeProperty('percent-pos', 50));

mpv.on('statuschange', states => {
    position = states['percent-pos'];
    console.log(position); // Will display 0.01% up to 99.99%, no problem here
});

mpv.on('quit', () => console.log(position)) // Will display 'null' instead of the last percent-pos known, as it used to on 0.29
Software Versions
  • Node-Mpv: branch 2, commit 6fc6037 (latest at writing time)
  • MPV: 0.32.0 stable, windows build taken from the official recommended repository
Additional context

Might not be a node-mpv issue but a MPV issue? Trying to update from 0.29 has caused that problem mainly, afaik.

vankasteelj avatar Mar 11 '20 19:03 vankasteelj

The question is how would the fix look like? Output 100.0 or always save the previous value when the value is null? I'm not 100% sure if that wouldn't eventually cause some error at a different place and doesn't quite seem like an elegant fix.

j-holub avatar Mar 12 '20 15:03 j-holub

Currently, I save the last percent-pos elsewhere and update it if it's non-null only, so after exiting I have the last known percent-pos transmitted by mpv: https://github.com/vankasteelj/frak/commit/39e1e8bcdcff14fa27657615ef12db84a6185bc8

It's inelegant, but it works so far.

vankasteelj avatar Mar 15 '20 08:03 vankasteelj

I don't know if mpv introduced a new state to avoid that "null" percent-pos in their latest updates, if they decided to void that value, or what. But I know that node-mpv gives back correct "states" values after exiting, but not for the added observed-property percent-pos. It could be that observeProperty needs to be udpated in your code?

The thing is, I don't know where that "null" comes from, if it's from mpv or your code.

vankasteelj avatar Mar 15 '20 08:03 vankasteelj

I believe it has to do with how events are handled since MPV 0.31, that states in the release notes something like "deprecate timed events", because as of >0.30 atm pause/resume events are fired twice.

vankasteelj avatar Apr 19 '20 11:04 vankasteelj

Interessting. I will have to read into that. It's always a pain to deal with different MPV versions, because you never know which one people have installed. If you use the Verison from Ubuntu or Debian Distributions it's ancient...

j-holub avatar Apr 19 '20 14:04 j-holub

Has this issue ever occured again? Just asking to see if it's worth investing time in.

j-holub avatar Jan 14 '21 22:01 j-holub

Still using the workaround, so I'm guessing yeah, nothing changed on my side. But I havent tested without the workaround

vankasteelj avatar Jan 15 '21 19:01 vankasteelj

I used the following code to reproduce the issue (slightly adapted to the breaking changes I made with version 2)

mpv.start().then(() => mpv.load(<somefile>)).then( () => mpv.observeProperty('percent-pos', 50));

mpv.on('status', state => {
    if (state['property'] == 'percent-pos') {
        position = state['value'];
        console.log(position); // Will display 0.01% up to 99.99%, no problem here
    }
});

mpv.on('quit', () => console.log(position))

It does output 0, but it's undefined once before the song quits. I'm still unsure if this is an issue or not, if it causes problems or not. But I might have an idea I could publish as a PR. I'll notify you if I got one.

j-holub avatar Mar 14 '21 16:03 j-holub