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

loadstart should fire during the resource selection algorithm #93

Merged
merged 3 commits into from
May 23, 2014

Conversation

dmlap
Copy link
Member

@dmlap dmlap commented May 22, 2014

Previously, loadstart and waiting fired once the video started playing. This is later than specced for loadstart and waiting doesn't even make sense. Move loadstart to the moment when we create the NetConnection which is closer to the desired behavior. Remove the waiting event from this flow.

Previously, loadstart and waiting fired once the video started playing. This is later than specced for loadstart and waiting doesn't even make sense. Move loadstart to the moment when we create the NetConnection which is closer to the desired behavior. Remove the waiting event from this flow.
@@ -402,6 +402,10 @@ package com.videojs.providers{
}

private function initNetConnection():void{
// the video element triggers loadstart as soon as the resource selection algorithm selects a source
// this is somewhat later than that moment but relatively close
ExternalInterface.call('console.trace');
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need this trace?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops.

Get rid of console.trace()
@heff
Copy link
Member

heff commented May 22, 2014

Makes sense to me.

Calling abort() on a source buffer resets the decoder buffers. Instead of relying on this happening implicitly through seeking, add a method so that media sources can trigger it directly.

public function abort():void{
// flush the netstream buffers
_ns.seek(time);
Copy link
Member

Choose a reason for hiding this comment

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

should time be here?

Copy link
Member

Choose a reason for hiding this comment

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

It depends on whether the netstream requires an input value or not. We're basically forcing it to flush the buffers by calling seek to the current time. It's possible we could just call _ns.seek() but having time there shouldn't make a difference, really.

Copy link
Contributor

Choose a reason for hiding this comment

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

NetStream.seek requires an offset time as a parameter ( per API reference: http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/net/NetStream.html#seek() ). Buffer flushing is normalized as NetStream.seek(currentTime).

Copy link
Member

Choose a reason for hiding this comment

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

But where is time defined? It's at least not clear from what I'm looking at.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

If that's the time it's referring to, should it be _ns.seek(time()) then? Or do class property getters always execute? I really don't know action script...

Copy link
Contributor

Choose a reason for hiding this comment

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

This syntax is correct within actionscript when the originating function is explicitly set as a getter. Least that's how I remember it, it's been a while since I was in here :)

Copy link
Member

Choose a reason for hiding this comment

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

ha, ok. ignore me then

Copy link
Member

Choose a reason for hiding this comment

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

Heh, yeah, we had this problem also.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think getters are a terrible language feature in general because they're so easy to abuse but when in Rome...

dmlap added a commit that referenced this pull request May 23, 2014
loadstart should fire during the resource selection algorithm
@dmlap dmlap merged commit 31d78a8 into master May 23, 2014
@dmlap dmlap deleted the hotfix/loadstart branch May 23, 2014 15:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants