Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Rename _isPlaying to _playbackStarted. #144

Merged
merged 1 commit into from
Mar 17, 2015
Merged

Rename _isPlaying to _playbackStarted. #144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified dist/video-js.swf
View file
Open in desktop
Binary file not shown.
34 changes: 17 additions & 17 deletions src/com/videojs/providers/HTTPVideoProvider.as
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ package com.videojs.providers{

private var _src:Object;
private var _metadata:Object;
private var _isPlaying:Boolean = false;
private var _playbackStarted:Boolean = false;
private var _isPaused:Boolean = true;
private var _isBuffering:Boolean = false;
private var _isSeeking:Boolean = false;
Expand Down Expand Up @@ -117,7 +117,7 @@ package com.videojs.providers{
// if we have metadata and a known duration
if(_metadata != null && _metadata.duration != undefined){
// if playback has begun
if(_isPlaying){
if(_playbackStarted){
// if the asset can play through without rebuffering
if(_canPlayThrough){
return 4;
Expand Down Expand Up @@ -213,7 +213,7 @@ package com.videojs.providers{
}

public function get playing():Boolean{
return _isPlaying;
return _playbackStarted;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

playbackStarted to me sounds like the video has started playing but may currently be paused or playing. Is that what this is meant to be?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that's it exactly and why I thought the original name was misleading...

@heff this line in particular was confusing to read.
https://github.com/videojs/video-js-swf/blob/master/src/com/videojs/providers/HTTPVideoProvider.as#L303

Perhaps this isn't a good name either though...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, from what @bclwhitaker and I understood, this is what this variable actually means. It's true from when a user as pressed play/seeked and until the video has ended.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. The playing() function is pretty poorly named too then. Thanks for clearing that up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bclwhitaker yeah _isPlaying && _isPaused is ridiculous. In core we have the vjs-has-started class if you're looking for similar naming, though I've considered changing that to vjs-played to match vjs-ended and the video element's played property. What you have here is good though.

}

public function get paused():Boolean{
Expand Down Expand Up @@ -260,15 +260,15 @@ package com.videojs.providers{

public function load():void{
_pauseOnStart = true;
_isPlaying = false;
_playbackStarted = false;
initNetConnection();
}

public function play():void{
// if this is a fresh playback request
if(!_loadStarted){
_pauseOnStart = false;
_isPlaying = false;
_playbackStarted = false;
_metadata = {};
initNetConnection();
}
Expand All @@ -291,7 +291,7 @@ package com.videojs.providers{
public function pause():void{
var alreadyPaused = _isPaused;
_ns.pause();
if(!alreadyPaused){
if(_playbackStarted && !alreadyPaused){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the important bit.

_model.broadcastEventExternally(ExternalEventName.ON_PAUSE);
if(_isBuffering){
_pausePending = true;
Expand All @@ -300,7 +300,7 @@ package com.videojs.providers{
}

public function resume():void{
if(_isPlaying && _isPaused){
if(_playbackStarted && _isPaused){
_ns.resume();
_model.broadcastEventExternally(ExternalEventName.ON_RESUME);
if (!_isBuffering) {
Expand All @@ -310,7 +310,7 @@ package com.videojs.providers{
}

public function seekBySeconds(pTime:Number):void{
if(_isPlaying)
if(_playbackStarted)
{
_isSeeking = true;
_throughputTimer.stop();
Expand All @@ -321,7 +321,7 @@ package com.videojs.providers{
}
else if(_hasEnded)
{
_isPlaying = true;
_playbackStarted = true;
_hasEnded = false;
}

Expand All @@ -338,7 +338,7 @@ package com.videojs.providers{
}

public function seekByPercent(pPercent:Number):void{
if(_isPlaying && _metadata.duration != undefined){
if(_playbackStarted && _metadata.duration != undefined){
_isSeeking = true;
if(pPercent < 0){
_ns.seek(0);
Expand All @@ -356,9 +356,9 @@ package com.videojs.providers{
}

public function stop():void{
if(_isPlaying){
if(_playbackStarted){
_ns.close();
_isPlaying = false;
_playbackStarted = false;
_hasEnded = true;
_model.broadcastEvent(new VideoPlaybackEvent(VideoPlaybackEvent.ON_STREAM_CLOSE, {}));
_throughputTimer.stop();
Expand Down Expand Up @@ -524,7 +524,7 @@ package com.videojs.providers{

case "NetStream.Buffer.Full":
_pausedSeekValue = -1;
_isPlaying = true;
_playbackStarted = true;
if(_pausePending){
_pausePending = false;
_ns.pause();
Expand All @@ -537,13 +537,13 @@ package com.videojs.providers{

case "NetStream.Buffer.Empty":
// should not fire if ended/paused. issue #38
if(!_isPlaying){ return; }
if(!_playbackStarted){ return; }

// reaching the end of the buffer after endOfStream has been called means we've
// hit the end of the video
if (_ending) {
_ending = false;
_isPlaying = false;
_playbackStarted = false;
_isPaused = true;
_hasEnded = true;
_model.broadcastEvent(new VideoPlaybackEvent(VideoPlaybackEvent.ON_STREAM_CLOSE, {info:e.info}));
Expand All @@ -561,7 +561,7 @@ package com.videojs.providers{

case "NetStream.Play.Stop":
if(!_loop){
_isPlaying = false;
_playbackStarted = false;
_hasEnded = true;
_model.broadcastEvent(new VideoPlaybackEvent(VideoPlaybackEvent.ON_STREAM_CLOSE, {info:e.info}));
_model.broadcastEventExternally(ExternalEventName.ON_PAUSE);
Expand All @@ -576,7 +576,7 @@ package com.videojs.providers{
break;

case "NetStream.Seek.Notify":
_isPlaying = true;
_playbackStarted = true;
_isSeeking = false;
_model.broadcastEvent(new VideoPlaybackEvent(VideoPlaybackEvent.ON_STREAM_SEEK_COMPLETE, {info:e.info}));
_model.broadcastEventExternally(ExternalEventName.ON_SEEK_COMPLETE);
Expand Down