Skip to content

Commit

Permalink
Remove position property from Player
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acrisci authored and emersion committed Mar 5, 2019
1 parent ff86557 commit d38d2ed
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 25 deletions.
5 changes: 5 additions & 0 deletions examples/player.js
Expand Up @@ -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) {
Expand Down
28 changes: 19 additions & 9 deletions src/index.js
Expand Up @@ -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`.
Expand Down Expand Up @@ -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)) {
Expand All @@ -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);
Expand Down Expand Up @@ -203,16 +202,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) {
Expand Down
9 changes: 2 additions & 7 deletions src/interfaces/player.js
Expand Up @@ -25,7 +25,6 @@ class PlayerInterface extends MprisInterface {
_Rate = 1;
_Shuffle = false;
_Volume = 0;
_Position = 0;
_LoopStatus = 'None';
_PlaybackStatus = 'Stopped';

Expand Down Expand Up @@ -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'})
Expand Down Expand Up @@ -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'})
Expand Down
22 changes: 13 additions & 9 deletions test/player.test.js
Expand Up @@ -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();
Expand All @@ -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));
});

0 comments on commit d38d2ed

Please sign in to comment.