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

Make output_format matching more robust. #1

Merged
merged 2 commits into from
Jan 1, 2015

Conversation

edk
Copy link
Contributor

@edk edk commented Dec 31, 2014

The issue at ruby-av/paperclip-av-transcoder#3
is caused, at least on my system with Paperclip 4.2, by the format
parameter of the form: ".FMT". This caused the case statement to not
match any of the desired formats! It's likely that other paperclip
version don't pass the dot before the format extension, but it seems
likely that the behavior may differ depending on the version.

This change looks for the end of the format string to match using a
regex, and should now properly find the format, allowing thumbnails
to be generated (as well as the proper switches for non image params)

The issue at ruby-av/paperclip-av-transcoder#3
is caused, at least on my system with Paperclip 4.2, by the format
parameter of the form: ".FMT".  This caused the case statement to not
match any of the desired formats!  It's likely that other paperclip
version don't pass the dot before the format extension, but it seems
likely that the behavior may differ depending on the version.

This change looks for the end of the format string to match using a
regex, and should now properly find the format, allowing thumbnails
to be generated (as well as the proper switches for non image params)
@owahab
Copy link
Member

owahab commented Dec 31, 2014

Breaks the gem. https://travis-ci.org/ruby-av/av/builds/45563088

@edk
Copy link
Contributor Author

edk commented Jan 1, 2015

whoops. I'll look into that. thanks.

It was failing because it was manually adding the output_type,
while the Base#add_destination does it as well, causing the
options, aac and experimental to be added twice.

While this fixes the issue, I wonder if it might be better
to make the option setting idempontent?  Although with command-
line switches, I suppose one never knows when repeated arguments
are valid and necessary.
@edk
Copy link
Contributor Author

edk commented Jan 1, 2015

@owahab If fixed the failing spec. Please let me know what you think, thanks!

@owahab
Copy link
Member

owahab commented Jan 1, 2015

Awesome, works. Thank you.

owahab added a commit that referenced this pull request Jan 1, 2015
Make output_format matching more robust.
@owahab owahab merged commit 6f22092 into ruby-av:master Jan 1, 2015
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.

2 participants