Skip to content

Commit

Permalink
Fix #2108 - Fix gif uploads (#2171)
Browse files Browse the repository at this point in the history
* Fix #2108 - Fix gif uploads
Add specs for media attachment gifv conversion

* Add ffmpeg to travis

* Make travis install ffmpeg, not libav

* Switch travis to trusty
  • Loading branch information
Gargron authored Apr 19, 2017
1 parent 0876a06 commit 2e4afcc
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 10 deletions.
5 changes: 4 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
language: ruby
cache: bundler
dist: trusty
sudo: required

notifications:
email: false
Expand All @@ -24,8 +26,9 @@ bundler_args: --without development production --retry=3 --jobs=3

before_install:
- sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test
- sudo add-apt-repository -y ppa:mc3man/trusty-media
- sudo apt-get -qq update
- sudo apt-get -qq install g++-4.8
- sudo apt-get -qq install g++-4.8 ffmpeg
install:
- nvm install
- npm install -g yarn
Expand Down
21 changes: 13 additions & 8 deletions app/models/media_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,18 @@ def set_shortcode
end

def set_type_and_extension
if file.blank?
self.type = :unknown
else
self.type = VIDEO_MIME_TYPES.include?(file_content_type) ? :video : :image
extension = Paperclip::Interpolations.content_type_extension(file, :original)
basename = Paperclip::Interpolations.basename(file, :original)
file.instance_write :file_name, [basename, extension].delete_if(&:empty?).join('.')
end
self.type = VIDEO_MIME_TYPES.include?(file_content_type) ? :video : :image
extension = appropriate_extension
basename = Paperclip::Interpolations.basename(file, :original)
file.instance_write :file_name, [basename, extension].delete_if(&:empty?).join('.')
end

def appropriate_extension
mime_type = MIME::Types[file.content_type]

extensions_for_mime_type = mime_type.empty? ? [] : mime_type.first.extensions
original_extension = Paperclip::Interpolations.extension(file, :original)

extensions_for_mime_type.include?(original_extension) ? original_extension : extensions_for_mime_type.first
end
end
6 changes: 5 additions & 1 deletion lib/paperclip/gif_transcoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ class GifTranscoder < Paperclip::Processor
def make
num_frames = identify('-format %n :file', file: file.path).to_i

return file unless options[:style] == :original && num_frames > 1
unless options[:style] == :original && num_frames > 1
tmp_file = Paperclip::TempfileFactory.new.generate(attachment.instance.file_file_name)
tmp_file << file.read
return tmp_file
end

final_file = Paperclip::Transcoder.make(file, options, attachment)

Expand Down
Binary file added spec/fixtures/files/attachment.gif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 22 additions & 0 deletions spec/models/media_attachment_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
require 'rails_helper'

RSpec.describe MediaAttachment, type: :model do
describe 'animated gif conversion' do
let(:media) { MediaAttachment.create(account: Fabricate(:account), file: attachment_fixture('avatar.gif')) }

it 'sets type to gifv' do
expect(media.type).to eq 'gifv'
end

it 'converts original file to mp4' do
expect(media.file_content_type).to eq 'video/mp4'
end
end

describe 'non-animated gif non-conversion' do
let(:media) { MediaAttachment.create(account: Fabricate(:account), file: attachment_fixture('attachment.gif')) }

it 'sets type to image' do
expect(media.type).to eq 'image'
end

it 'leaves original file as-is' do
expect(media.file_content_type).to eq 'image/gif'
end
end
end

0 comments on commit 2e4afcc

Please sign in to comment.