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

M4V support #139

Closed
grdw opened this issue Sep 6, 2019 · 3 comments · Fixed by #144
Closed

M4V support #139

grdw opened this issue Sep 6, 2019 · 3 comments · Fixed by #144
Labels

Comments

@grdw
Copy link

grdw commented Sep 6, 2019

Follow up from #138


Currently in Spaceship we have a video thumbnailer. In Spaceship we check the file extension instead of actually using the format parser (mainly because changing the file extension on mobile is quite uncommon). However, we still have plans to use the format parser for this, if only it supported M4V formats.

Related: WeTransfer/spaceship#1975

@grdw grdw added the formats label Sep 6, 2019
@julik
Copy link
Contributor

julik commented Sep 14, 2019

Isn't MV4 a subset of MPEG4 and thus effectively MOV? The parser we have should be picking it up.

@julik
Copy link
Contributor

julik commented Sep 14, 2019

I.e. it should be supported via https://github.com/WeTransfer/format_parser/blob/master/lib/parsers/moov_parser.rb or it should be relatively within reach to make sure it is supported

@grdw
Copy link
Author

grdw commented Sep 16, 2019

Isn't MV4 a subset of MPEG4 and thus effectively MOV?

I didn't knew this, but I just learned that is the case indeed. It is like a MOV. I guess it requires some minor changes to the likely_match? method, to make sure the mov parser get's sorted on top of the list of parsers, but I guess that's it. It already should be supported indeed.

@julik julik closed this as completed in #144 Nov 2, 2019
julik pushed a commit that referenced this issue Nov 2, 2019
As m4v is already part of the mov family, it already parses this information successfully. However it's not included in the likely matches. This PR takes care of that.

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

Successfully merging a pull request may close this issue.

2 participants