From fd63b07290caf6553f4ff9e041e197fa4cbde7ac Mon Sep 17 00:00:00 2001 From: Tony Crisci Date: Tue, 5 Mar 2019 09:08:04 -0500 Subject: [PATCH] Remove `position` property from Player Rationale: this property is hard to support for implementing players as a property they must keep up-to-date because it continuously changes over time measured in microseconds. Player#seeked() now takes the absolute position instead of a delta for consistency with the mpris interface and because we no longer store position on the player to do this calculation. The 'seeked' event on the Player now emits with the offset to seek only for a similar reason. add Player#getPosition() to be overridden by the user to be called whenver this property is gotten by the properties interface. --- examples/player.js | 5 +++++ src/index.js | 28 +++++++++++++++++++--------- src/interfaces/player.js | 9 ++------- test/player.test.js | 22 +++++++++++++--------- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/examples/player.js b/examples/player.js index 01319b3..9d20ecf 100644 --- a/examples/player.js +++ b/examples/player.js @@ -8,6 +8,11 @@ var player = Player({ supportedInterfaces: ['player'] }); +player.getPosition = function() { + // return the position of your player + return 0; +} + // Events var events = ['raise', 'quit', 'next', 'previous', 'pause', 'playpause', 'stop', 'play', 'seek', 'position', 'open', 'volume', 'loopStatus', 'shuffle']; events.forEach(function (eventName) { diff --git a/src/index.js b/src/index.js index 51f2464..98a3971 100644 --- a/src/index.js +++ b/src/index.js @@ -39,7 +39,7 @@ function lcfirst(str) { * * `playPause` - Pauses playback. If playback is already paused, resumes playback. If playback is stopped, starts playback. * * `stop` - Stops playback. * * `play` - Starts or resumes playback. - * * `seek` - Seeks forward in the current track by the specified number of microseconds. With event data `{ delta, position }`. + * * `seek` - Seeks forward in the current track by the specified number of microseconds. With event data `offset`. * * `position` - Sets the current track position in microseconds. With event data `{ trackId, position }`. * * `open` - Opens the Uri given as an argument. With event data `{ uri }`. * * `activatePlaylist` - Starts playing the given playlist. With event data `playlistId`. @@ -75,7 +75,7 @@ function lcfirst(str) { * @property {Double} minimumRate - The minimum value which the Rate property can take. * @property {Double} maximumRate - The maximum value which the Rate property can take. * @property {Array} playlists - The current playlists set by Player#setPlaylists. (Not a DBus property). - * @property {Integer} activePlaylist - The currently-active playlist. + * @property {String} activePlaylist - The id of the currently-active playlist. */ function Player(opts) { if (!(this instanceof Player)) { @@ -86,7 +86,6 @@ function Player(opts) { this.name = opts.name; this.supportedInterfaces = opts.supportedInterfaces || ['player']; this._tracks = []; - this.position = 0; this.init(opts); } util.inherits(Player, events.EventEmitter); @@ -201,16 +200,27 @@ Player.prototype._addEventedPropertiesList = function(iface, props) { }; /** - * Sets the position of the player to `position + delta` and emits the `Seeked` - * DBus signal to listening clients. + * Gets the position of this player. This method is intended to be overridden + * by the user to return the position of the player in microseconds. + * + * @name Player#getPosition + * @function + * @returns {Integer} - The current position of the player in microseconds. + */ +Player.prototype.getPosition = function() { + return 0; +} + +/** + * Emits the `Seeked` DBus signal to listening clients with the given position. * * @name Player#seeked * @function - * @param {Integer} delta - The change in position in microseconds. + * @param {Integer} position - The position in microseconds. */ -Player.prototype.seeked = function(delta) { - this.position += delta || 0; - this.interfaces.player.Seeked(this.position); +Player.prototype.seeked = function(position) { + position = position || 0; + this.interfaces.player.Seeked(position); }; Player.prototype.getTrackIndex = function(trackId) { diff --git a/src/interfaces/player.js b/src/interfaces/player.js index 75c4716..c0bdf53 100644 --- a/src/interfaces/player.js +++ b/src/interfaces/player.js @@ -25,7 +25,6 @@ class PlayerInterface extends MprisInterface { _Rate = 1; _Shuffle = false; _Volume = 0; - _Position = 0; _LoopStatus = 'None'; _PlaybackStatus = 'Stopped'; @@ -106,7 +105,7 @@ class PlayerInterface extends MprisInterface { @property({signature: 'x', access: ACCESS_READ}) get Position() { - return this._Position; + return this.player.getPosition(); } @property({signature: 's'}) @@ -156,11 +155,7 @@ class PlayerInterface extends MprisInterface { Seek(offset) { // XXX overflow offset = JSBI.toNumber(offset); - let e = { - delta: offset, - position: (this.player.position || 0) + offset - }; - this.player.emit('seek', e); + this.player.emit('seek', offset); } @method({inSignature: 'ox'}) diff --git a/test/player.test.js b/test/player.test.js index 1330739..cb9b366 100644 --- a/test/player.test.js +++ b/test/player.test.js @@ -188,18 +188,23 @@ test('position specific properties, methods, and signals should work', async () return peer.Ping(); }; - // position starts at 0 + // position defaults to always being 0 let position = await props.Get(PLAYER_IFACE, 'Position'); expect(position).toEqual(new Variant('x', JSBI.BigInt(0))); + // when the getter is set, it should return what the getter returns + player.getPosition = function() { + return 99; + } + + position = await props.Get(PLAYER_IFACE, 'Position'); + expect(position).toEqual(new Variant('x', JSBI.BigInt(99))); + // Seek - let cb = jest.fn(e => { - player.seeked(e.delta); - }); + let cb = jest.fn(); player.once('seek', cb); await playerIface.Seek(99); - expect(cb).toHaveBeenCalledWith({ delta: 99, position: 99 }); - expect(player.position).toEqual(99); + expect(cb).toHaveBeenCalledWith(99); // SetPosition cb = jest.fn(); @@ -209,8 +214,7 @@ test('position specific properties, methods, and signals should work', async () cb = jest.fn(); playerIface.once('Seeked', cb); - player.seeked(99); + player.seeked(200); await ping(); - // this one updates position - expect(cb).toHaveBeenCalledWith(JSBI.BigInt(198)); + expect(cb).toHaveBeenCalledWith(JSBI.BigInt(200)); });