Fix #2108 - Fix gif uploads (#2171)

* 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

diff --git a/.travis.yml b/.travis.yml
index 564155c662d243558cac33b225f637ba340f6aab..9725f4993aed9f25e86ec724d3d54ccaf4ffd537 100644 (file)
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,5 +1,7 @@
 language: ruby
 cache: bundler
+dist: trusty
+sudo: required
 
 notifications:
   email: false
@@ -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

diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index bb16adb5bcdb82a13a5d5bea492be4f3a714dbc0..181e01674f34d97c5d1be35087be10876b38e127 100644 (file)
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -106,13 +106,18 @@ class MediaAttachment < ApplicationRecord
   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

diff --git a/lib/paperclip/gif_transcoder.rb b/lib/paperclip/gif_transcoder.rb
index 8337448b2374a2838e10bed9088f29a9a56ed4a3..cbe53b27b35f6529f1144b15f03974e4f7f5b329 100644 (file)
--- a/lib/paperclip/gif_transcoder.rb
+++ b/lib/paperclip/gif_transcoder.rb
@@ -7,7 +7,11 @@ module Paperclip
     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)
 

diff --git a/spec/fixtures/files/attachment.gif b/spec/fixtures/files/attachment.gif
new file mode 100644 (file)
index 0000000..2937f5a
Binary files /dev/null and b/spec/fixtures/files/attachment.gif differ

diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb
index 5995aa4f4e098cd3e76e4a836ac062cc0f4df239..d98f7e696bf3b11c7f0ea91cb72a23ac7c847b78 100644 (file)
--- a/spec/models/media_attachment_spec.rb
+++ b/spec/models/media_attachment_spec.rb
@@ -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