j-holub / Node-MPV

A NodeJs Module for MPV Player
https://www.npmjs.com/package/node-mpv
MIT License
116 stars 73 forks source link

'seek' event #23

Closed vankasteelj closed 7 years ago

vankasteelj commented 7 years ago

How about something like mpv.on('seek', newPosition) ?

it's trivial to make, but there are 2 ways of doing it.

1) There is the simple 'seek' event, you can add to your V2 /lib/mpv/_events.js, but I don't really see the point, because you can't attach anything to that event, the seek isnt done yet, MPV is just saying "hey i'm seeking, come back later to check where to":

                case "seek":
                    if(this.options.verbose){console.log("Event: seek")};
                    // emit seek event
                    this.emit("seek");
                    break;

2) A more advanced seeking, emitting the new time position, but again, there is something I don't like, as it will emit statuschange at the same time (and it should, but it seems like an overkill of events).

            case "seek":
                if(this.options.verbose){console.log("Event: seek")};
                this.wasSeek = true;
                break;
            // observed properties
            case "property-change":
                // time position events are handled seperately
                if(message.name === "time-pos"){
                    // set the current time position
                    this.currentTimePos = message.data;
                }
                else{
                    // updates the observed value or adds it, if it was previously unobserved
                    this.observed[message.name] = message.data;
                    // emit a status change event
                    this.emit('statuschange', this.observed);
                    // output if verbose
                    if(this.options.verbose){
                        console.log("Event: statuschange");
                        console.log("Property change: " + message.name + " - " + message.data);
                    }
                }
                if (this.wasSeek){
                     this.emit("seek", this.currentTimePos);
                     this.wasSeek = false;
                }
                break;

There should be a third option, imho, but I can't seem to think of it.

vankasteelj commented 7 years ago

there is, of course, the possibility for the developer to mpv.observeProperty('seeking', id), but that's not ideal, as it will also trigger a seek when the player starts (maybe because internally, mpv seeks to 0 when loading a file?)

That's what I currently use, it's working, as this:

        Player.mpv.on('statuschange', states => {
            if (states.seeking) {
                // here I report to the main js script that the playback has started to 0 or the user made a seek
            }
        });
j-holub commented 7 years ago

I have already found and experimented with the seekingproperty and it worked quite well. I save the current timeposition anyway so I just output the saved value. Which worked in most cases.

As you already figured and stated in the mpv documentation, the seeking event is also fired when a file is loaded. The time position is set to null however, which is something, that could be used to indicated whether the user is really seeking or the file is just loading the file.

It sounds like it will cause a lot of race conditions though, but I think that could be the way to do it. I would definitely prefer adding another event and not include this into the status object. Though doing so will add the nice functionality to have the timeposition and seek event at the same time. I will have a look, definitely sounds like a great addition to the module

j-holub commented 7 years ago

I was experimenting a little further and found out, that after the seek is finished, mpv emits a so called playback-restart event. I can use this to find out when the seek was finished.

I went a different approach and added a promise within the seek method, that watches the output over a temporary socket and resolves on seek completion with both the starting and the ending time of the seeking.

So far I'm rather pleased with it, works quite fine, though the starting position is still a little buggy. If I can't work it out at all I'll just omit it. I thought it is nice addition.

bildschirmfoto 2017-08-17 um 23 26 11

Maybe I can still somehow fiddle it inside the normal event handler, I'll work it out. It crashed then I do the following

player.seek(100);
player.loadfile("some/file.mp3");

but that is due to the message handling. It's not done as sufficient as in the usual message handler of the ipcInterface class. I'll work it out. But I can use tracks-changed events for that (which would be a nice addition as an event for this module I think) to check for that stuff.

Looking good so far, this is how the output looks by the way

bildschirmfoto 2017-08-17 um 23 26 57
j-holub commented 7 years ago

I could fix the starting time problem, however I realized, that if I do it this way, the event is not fired when someone else is seeking, only if you seek through the module. Guess I'll have to do it in a different way.

j-holub commented 7 years ago

I added a seek event and for me it works so far. I am not 100% sure there are no edge cases were something goes wrong. Please try it out and see if it works for you as intended.

It is also called when the goToPosition() method is used, which makes sense to me.

Use it like this

player.on('seek', (timepositions) => {
    console.log("Seek: " + timepositions.start + " - " + timepositions.end);
}

though I'm thinking about just passing two separate variables into the event instead of a JSON object, so that is subject to change before I merge version 2 into master.