Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended canHandleSource in media files to consistently guess mime types. Fixes #1833 #1974

Merged
merged 2 commits into from
Apr 2, 2015
Merged

Extended canHandleSource in media files to consistently guess mime types. Fixes #1833 #1974

merged 2 commits into from
Apr 2, 2015

Conversation

carpasse
Copy link
Contributor

Added getFileExtension to libs. And used it on Html5 and Flash canHandleSource method to guess the mime type if necessary.

@heff
Copy link
Member

heff commented Mar 24, 2015

Awesome, thanks!

var pathParts;

if(typeof path === 'string'){
splitPathRe = /^(\/?)([\s\S]*?)((?:\.{1,2}|[^\/]+?)(\.([^\.\/\?]+)))(?:[\/]*|[\?].*)$/i;
Copy link
Member

Choose a reason for hiding this comment

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

That's a pretty advanced regex. What does it add over the one that was in the html5 tech?

Copy link
Member

Choose a reason for hiding this comment

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

It's the regex from iojs, which we talked about in the other pr comments

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right. Thanks.

@heff
Copy link
Member

heff commented Mar 24, 2015

Added a few comments, but otherwise this looks good to me. 👍

@heff
Copy link
Member

heff commented Mar 25, 2015

I think this is ready to go. Just need one more person to review and sign off on it. @videojs/core-committers

@xy2015
Copy link

xy2015 commented Mar 28, 2015

Sent from Windows Mail

From: Steve Heffernan
Sent: ‎Wednesday‎, ‎March‎ ‎25‎, ‎2015 ‎7‎:‎25‎ ‎AM
To: videojs/video.js

Added a few comments, but otherwise this looks good to me. 👍


Reply to this email directly or view it on GitHub.

@carpasse
Copy link
Contributor Author

@xy2015 I can not see you comments can you please add them again or point me to them??

@dmlap
Copy link
Member

dmlap commented Apr 1, 2015

LGTM

dmlap added a commit that referenced this pull request Apr 2, 2015
Extended canHandleSource in media files to consistently guess mime types. Fixes #1833
@dmlap dmlap merged commit 7a9c4cc into videojs:master Apr 2, 2015
@dmlap
Copy link
Member

dmlap commented Apr 2, 2015

@heff I hit the "merge" button by habit, sorry. I'll fix up the changelog tomorrow morning

@heff
Copy link
Member

heff commented Apr 2, 2015

No problem. Though @mmcc might not have fun rebasing the ES6 work again. 😄

@mmcc
Copy link
Member

mmcc commented Apr 2, 2015

(╯°□°)╯︵ ┻━┻ 


Sent from mobile

On Wed, Apr 1, 2015 at 9:30 PM, Steve Heffernan notifications@github.com
wrote:

No problem. Though @mmcc might not have fun rebasing the ES6 work again. 😄

Reply to this email directly or view it on GitHub:
#1974 (comment)

@dmlap
Copy link
Member

dmlap commented Apr 2, 2015

@mmcc sorry!

dmlap added a commit to dmlap/video.js that referenced this pull request Apr 2, 2015
@dmlap dmlap mentioned this pull request Apr 2, 2015
heff added a commit that referenced this pull request Apr 2, 2015
@mmcc
Copy link
Member

mmcc commented Apr 2, 2015

Ok this is now pulled into ES6 branch, but I'm going to hold off on pushes until all the comments are addressed. I've rebased from master to clean up the commit history so I think we'll lose all the comments when I end up force pushing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants