Added stopOthers similar to pauseOthers #139

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@harvest

harvest commented Feb 5, 2013

No description provided.

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Oct 30, 2013

Contributor

I'm not sure about this one... On desktops I see no issue, but on mobile browsers this might cause problems where the media that just started playing tried to stop the others, which is a pause(0), which attempts to set currentTime to zero... And on mobile browsers, they tend to only support a single instance at one time.

Agreed, since iOS6 you can play 2 or more audio together, but you cannot play 2 videos together.

I will think about it... But might maintain the pauseOthers() but add an optional parameter that you could put a 0 in to do the same as stop... Or put a pauseOthers(15) to make them all pause at 15 seconds... But that might actually be worse LOL - So hmm...

Contributor

ghost commented Oct 30, 2013

I'm not sure about this one... On desktops I see no issue, but on mobile browsers this might cause problems where the media that just started playing tried to stop the others, which is a pause(0), which attempts to set currentTime to zero... And on mobile browsers, they tend to only support a single instance at one time.

Agreed, since iOS6 you can play 2 or more audio together, but you cannot play 2 videos together.

I will think about it... But might maintain the pauseOthers() but add an optional parameter that you could put a 0 in to do the same as stop... Or put a pauseOthers(15) to make them all pause at 15 seconds... But that might actually be worse LOL - So hmm...

@harvest

This comment has been minimized.

Show comment Hide comment
@harvest

harvest Oct 31, 2013

@happyworm Purpose of stopOthers is to on-demand stop peer players on pages where there are multiple instances. For example, a bunch of preview players on a single page. e.g http://www.musicloops.com/

harvest commented Oct 31, 2013

@happyworm Purpose of stopOthers is to on-demand stop peer players on pages where there are multiple instances. For example, a bunch of preview players on a single page. e.g http://www.musicloops.com/

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Oct 31, 2013

Contributor

But that is what pauseOthers does. All stopOthers would do is pause the other players AND set their currentTime to zero. It is that last part that I have issues with.

Contributor

ghost commented Oct 31, 2013

But that is what pauseOthers does. All stopOthers would do is pause the other players AND set their currentTime to zero. It is that last part that I have issues with.

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Oct 31, 2013

Contributor

Also, that link you gave does not appear to use jPlayer. Those players were flash based - unless I missed something.

Contributor

ghost commented Oct 31, 2013

Also, that link you gave does not appear to use jPlayer. Those players were flash based - unless I missed something.

@harvest

This comment has been minimized.

Show comment Hide comment
@harvest

harvest Oct 31, 2013

@happyworm We wanted to get rid of the flash-based players to make it iOS friendly and that is when the requirement came up. If someone wants the pauseOthers to pause at their current state then it might cause an issue. If it is going to reset to 0 then that's fine.

harvest commented Oct 31, 2013

@happyworm We wanted to get rid of the flash-based players to make it iOS friendly and that is when the requirement came up. If someone wants the pauseOthers to pause at their current state then it might cause an issue. If it is going to reset to 0 then that's fine.

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Oct 31, 2013

Contributor

OK. I'm convinced... Consider this feature as considered.
After all, it is up to the dev whether they use this command and your case gives a good example of a minimal GUI with a play/stop button where you always want it to start playing from the start.

However... You may have been able to use this by adding a pause event handler via the options:

pause: function() {
   $(this).jPlayer("pause", 0);
}

Whenever the jPlayer was paused or stopped, it would reset to the start. And if it was playing when another was started, then the pauseOther would cause it to reset to the start.

I think I'll add the stopOthers command and test it on iOS with audio and video. I still have a bad feeling about that pause event handler if we were talking about video... Since once you start playing another video, the previous video kinda becomes lifeless on iOS. That is until you tell it to play again. I'll have to test and see.

Cheers for the discussion and PR.

Contributor

ghost commented Oct 31, 2013

OK. I'm convinced... Consider this feature as considered.
After all, it is up to the dev whether they use this command and your case gives a good example of a minimal GUI with a play/stop button where you always want it to start playing from the start.

However... You may have been able to use this by adding a pause event handler via the options:

pause: function() {
   $(this).jPlayer("pause", 0);
}

Whenever the jPlayer was paused or stopped, it would reset to the start. And if it was playing when another was started, then the pauseOther would cause it to reset to the start.

I think I'll add the stopOthers command and test it on iOS with audio and video. I still have a bad feeling about that pause event handler if we were talking about video... Since once you start playing another video, the previous video kinda becomes lifeless on iOS. That is until you tell it to play again. I'll have to test and see.

Cheers for the discussion and PR.

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Oct 31, 2013

Contributor

I might even make a generic tellOthers(command) method so the existing pauseOthers() method becomes something like:

pauseOthers: function() {
   tellOthers("pause");
}

The tellOthers(cmd) code would be almost identical to the current pauseOthers code. But should have the ability to take in more arguments.

Then you could use the new tellOthers("stop"); command... Or in the correct syntax:

$(this).jPlayer("tellOthers", "stop");

I'm thinking this way since I have hopes of making the volume shared across multiple instances (as an option) and this might become a useful common function that has more uses than I can imagine.

OT: The problem with the shared volume is that if each instance has its own volume controls, then I see infinite loops happening... I'll have to be careful to distinguish between the one that had the volume changed and the other instances. Anywayz - wandered off topic.

Contributor

ghost commented Oct 31, 2013

I might even make a generic tellOthers(command) method so the existing pauseOthers() method becomes something like:

pauseOthers: function() {
   tellOthers("pause");
}

The tellOthers(cmd) code would be almost identical to the current pauseOthers code. But should have the ability to take in more arguments.

Then you could use the new tellOthers("stop"); command... Or in the correct syntax:

$(this).jPlayer("tellOthers", "stop");

I'm thinking this way since I have hopes of making the volume shared across multiple instances (as an option) and this might become a useful common function that has more uses than I can imagine.

OT: The problem with the shared volume is that if each instance has its own volume controls, then I see infinite loops happening... I'll have to be careful to distinguish between the one that had the volume changed and the other instances. Anywayz - wandered off topic.

@harvest

This comment has been minimized.

Show comment Hide comment
@harvest

harvest Oct 31, 2013

That'll be great.

Cheers,
guru

harvest commented Oct 31, 2013

That'll be great.

Cheers,
guru

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Nov 2, 2013

Contributor

As always, things are always more complex than we remember, but I have a nice tellOthers(command[,conditions][,args]) method coming along.

I have been using it to issue the pauseOthers(0) command and have tested it on iOS. While the video players ignore the currentTime=0 instruction, it does not seem to cause any problems. The audio players seem to work fine with the command.

As for the conditions parameter of pauseOthers(), it is an optional function that returns true or false, which I'm using to maintain the current pauseOthers() functionality so that it does not tell the other instance to pause, if the other instance does not have its status.srcSet flag true. Otherwise it would generate an error event, which might cause confusion.

The only thing atm that is bugging me, is the context and information available to the conditions function.

My current code has the function's this as the other instance's jPlayer object. For example:

function() {
  return this.status.srcSet;
}

But now I'm wondering if we want to have easy access the jPlayer instance issuing the command's properties too... But maybe I'm over thinking the situation, and in practice it would be easy enough to pass that info into the function by using variable scope.

My thoughts were along the lines of either:

function(other) {
  return this.options.muted && other.options.muted;
}

Or:

function(master) {
  return master.options.muted && this.options.muted;
}

I think the latter one... this applies to the other instance and they can access the instance giving the orders through the parameter if they want.

Contributor

ghost commented Nov 2, 2013

As always, things are always more complex than we remember, but I have a nice tellOthers(command[,conditions][,args]) method coming along.

I have been using it to issue the pauseOthers(0) command and have tested it on iOS. While the video players ignore the currentTime=0 instruction, it does not seem to cause any problems. The audio players seem to work fine with the command.

As for the conditions parameter of pauseOthers(), it is an optional function that returns true or false, which I'm using to maintain the current pauseOthers() functionality so that it does not tell the other instance to pause, if the other instance does not have its status.srcSet flag true. Otherwise it would generate an error event, which might cause confusion.

The only thing atm that is bugging me, is the context and information available to the conditions function.

My current code has the function's this as the other instance's jPlayer object. For example:

function() {
  return this.status.srcSet;
}

But now I'm wondering if we want to have easy access the jPlayer instance issuing the command's properties too... But maybe I'm over thinking the situation, and in practice it would be easy enough to pass that info into the function by using variable scope.

My thoughts were along the lines of either:

function(other) {
  return this.options.muted && other.options.muted;
}

Or:

function(master) {
  return master.options.muted && this.options.muted;
}

I think the latter one... this applies to the other instance and they can access the instance giving the orders through the parameter if they want.

ghost pushed a commit that referenced this pull request Nov 3, 2013

@ghost ghost assigned happyworm Nov 3, 2013

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Nov 3, 2013

Contributor

Okay, this is now implemented and the commit in [dev] was tagged with this PR number. See above.

So the stopOthers() command is simply:

play: function() { // Stop other jPlayer instances
  $(this).jPlayer("pauseOthers", 0);
}

Or you can do it like this if you do not care about media errors, or if you setMedia on all your players to something:

play: function() { // Stop other jPlayer instances
  $(this).jPlayer("tellOthers", "stop");
}

Or if you do care:

play: function() { // Stop other jPlayer instances
  $(this).jPlayer("tellOthers", "stop", function() {
    return this.status.srcSet;
  });
}
Contributor

ghost commented Nov 3, 2013

Okay, this is now implemented and the commit in [dev] was tagged with this PR number. See above.

So the stopOthers() command is simply:

play: function() { // Stop other jPlayer instances
  $(this).jPlayer("pauseOthers", 0);
}

Or you can do it like this if you do not care about media errors, or if you setMedia on all your players to something:

play: function() { // Stop other jPlayer instances
  $(this).jPlayer("tellOthers", "stop");
}

Or if you do care:

play: function() { // Stop other jPlayer instances
  $(this).jPlayer("tellOthers", "stop", function() {
    return this.status.srcSet;
  });
}
@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Nov 3, 2013

Contributor

Release notes (the actual page has links and stuff):

  • [dev] New Feature: Added tellOthers(command[,conditions][,args]) method. This method enables an instance to issue commands to the other instances. The optional conditions(master) function executes with the other instance's context (this) and returns true or false, controlling whether the other instance is commanded. The master parameter is the jPlayer instance that is issuing the commands. Both this and master are the jPlayer JavaScript object. For example: this.status.srcSet or master.options.muted.
  • [dev] New Feature: Added time parameter to pauseOthers(time) method. This feature was proposed here Added stopOthers similar to pauseOthers by harvest

Closing this issue.
Cheers

Contributor

ghost commented Nov 3, 2013

Release notes (the actual page has links and stuff):

  • [dev] New Feature: Added tellOthers(command[,conditions][,args]) method. This method enables an instance to issue commands to the other instances. The optional conditions(master) function executes with the other instance's context (this) and returns true or false, controlling whether the other instance is commanded. The master parameter is the jPlayer instance that is issuing the commands. Both this and master are the jPlayer JavaScript object. For example: this.status.srcSet or master.options.muted.
  • [dev] New Feature: Added time parameter to pauseOthers(time) method. This feature was proposed here Added stopOthers similar to pauseOthers by harvest

Closing this issue.
Cheers

@ghost ghost closed this Nov 3, 2013

@harvest

This comment has been minimized.

Show comment Hide comment
@harvest

harvest Nov 3, 2013

Thank you.

harvest commented Nov 3, 2013

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment