Skip to content
This repository has been archived by the owner. It is now read-only.

Removed dependency on deprecated Youtube data API v2. #441

Merged
merged 2 commits into from Jun 5, 2015

Conversation

@syl22-00
Copy link
Contributor

syl22-00 commented May 7, 2015

This is a fix for issue #440 . Instead of using Youtube's data API, duration can be retrieved from the player itself.

'onStateChange': onPlayerStateChange
}
});
});

impl.networkState = self.NETWORK_LOADING;
self.dispatchEvent( "loadstart" );
self.dispatchEvent( "progress" );

This comment has been minimized.

@alidinani

alidinani Jun 4, 2015

This should be shifted over to the left. But beyond that, thanks 👍

@stevemao
Copy link
Contributor

stevemao commented Jun 4, 2015

This needs to be quickly merged as YouTube is not working at all.

@syl22-00
Copy link
Contributor Author

syl22-00 commented Jun 4, 2015

Indentation fix is done.

@cadecairos
Copy link
Member

cadecairos commented Jun 5, 2015

I'm not sure why the data api was used initially, perhaps @ScottDowne can speak to that, but why not update it to use the latest V3 API? https://developers.google.com/youtube/v3/docs/videos#contentDetails.duration

@mjschranz
Copy link
Contributor

mjschranz commented Jun 5, 2015

99% certain the original reason was to ensure proper duration data because of all the inconsistencies they have had. Perhaps things are better now?

@syl22-00
Copy link
Contributor Author

syl22-00 commented Jun 5, 2015

Youtube data API v3 seems to require an API token or authentication, and there is a usage quota. So it might not be great to rely on it.

@mjschranz
Copy link
Contributor

mjschranz commented Jun 5, 2015

Perhaps. I'll poke/prod at @ScottDowne to try and remember why (although I think I'm correct) and hopefully we can get something fixed soon.

ScottDowne added a commit that referenced this pull request Jun 5, 2015
Removed dependency on deprecated Youtube data API v2.
@ScottDowne ScottDowne merged commit 8ae202a into mozilla:master Jun 5, 2015
@oligibson
Copy link

oligibson commented Jun 6, 2015

@ScottDowne can this merge be tagged as anyone using bower to manage their versions won't get this fix.

@westis
Copy link

westis commented Jun 8, 2015

Is there any way to update manually with a fix for YouTube, before there is another release? We depend on this for a solution for city council meetings, and they are wondering why it's not working.

@ScottDowne
Copy link
Contributor

ScottDowne commented Jun 8, 2015

I tagged v1.5.11 for the youtube fix.

@westis
Copy link

westis commented Jun 8, 2015

Thanks. I suppose I then need to compile popcorn.min.js with the make-file somehow? For a "beginner" it's not quite clear what I need to do, as simply downloading the library did not work, but I still need a popcorn.min.js file. I'd be really grateful for some guidance.

@kreynen
Copy link

kreynen commented Jun 9, 2015

Downloading "the latest" release of Popcorn.js is confusing. Latest Release tag is pinned to 1.5.2 on GitHub. When users visit https://github.com/mozilla/popcorn-js/releases, they have to know to click on the ". . . 9 newer tags" to see any releases after that.

selection_031

http://popcornjs.org/download points to 1.5.6.

@mjschranz
Copy link
Contributor

mjschranz commented Jun 9, 2015

I'm not really sure why that is happening in the Github UI as the newer tags clearly exist.

I don't believe we ever will get an up to date http://popcornjs.org website so the main means for pulling in the script files now will either be downloading the zip from Github itself or from bower and then running the Make file.

In truth we really should just include a minified version in the root of the repo for people to make the bower use case much simpler.

@kreynen
Copy link

kreynen commented Jun 9, 2015

Releases and tags are not the same on GitHub. If the 1.5.2 "release" is deleted, all of the tags will be listed with the newest on top. If you don't want to delete that, someone had to add a release for each tag.

https://help.github.com/articles/editing-and-deleting-releases/

@nidhisarasan

This comment has been minimized.

Copy link

nidhisarasan commented on 925d1c5 Jun 10, 2015

this is not working.still it shows the error

@cemerson
Copy link

cemerson commented May 30, 2016

Can anyone help a Makefile noob get a build of 1.5.11 (or close)? I've wasted 2 hours with cygwin, mingw, vstudio command prompt etc and just can't get this Makefile to run (closest I get is some Interrupt/Exception error after a handful of File not founds for popcorn.*.unit.js. Super frustrating! Anyone care to help a noob out? I'd use that build tool but it only uses 1.5.6 and doesn't seem to have this YouTube fix.

@cadecairos
Copy link
Member

cadecairos commented May 30, 2016

@cemerson I've got an old build tool running here: http://popcorn-js.chrisdecairos.ca/ but it's building v1.5.7

@cemerson
Copy link

cemerson commented May 30, 2016

Thanks @cadecairos I was trying that ... but I'm not sure if the YouTube fix is in 1.5.7. Initial tests look like it's not.

@cadecairos
Copy link
Member

cadecairos commented May 30, 2016

I'll see if I can get an updated version up there

@cemerson
Copy link

cemerson commented May 30, 2016

Thank you @cadecairos - much appreciated!

@cadecairos
Copy link
Member

cadecairos commented May 30, 2016

@cemerson okay, http://popcorn-js.chrisdecairos.ca/ is now serving up popcorn-js v1.5.11

I even took the time to update all the apps dependencies, so it's using the latest uglify-js to minify.

source code of the build tool is here: https://github.com/cadecairos/popcorn-build-tool

@cemerson
Copy link

cemerson commented May 30, 2016

@cadecairos you are my hero man! Thank you! I can confirm YouTube happiness w/this 1.5.11 version! (at least so far). Again - thank you sir! All of us noobs who can't figure out Makefile stuff owe you!

@cadecairos
Copy link
Member

cadecairos commented May 30, 2016

Don't worry, makefiles are the worst - especially on Windows.

Popcorn should really be using a node powered build system...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.