Browse Source

Add customizable thumbnails for audio and video attachments (#14145)

- Change audio files to not be stripped of metadata
- Automatically extract cover art from audio if it exists
- Add `thumbnail` parameter to `POST /api/v1/media`, `POST /api/v2/media` and `PUT /api/v1/media/:id`
- Add `icon` to represent it in attachments in ActivityPub
- Fix `preview_url` containing URL of missing missing image when there is no thumbnail instead of null
- Fix duration of audio not being displayed on public pages until the file is loaded
Eugen Rochko 3 years ago
parent
commit
64aac30733

+ 1 - 1
app/controllers/api/v1/media_controller.rb

@@ -39,7 +39,7 @@ class Api::V1::MediaController < Api::BaseController
   end
 
   def media_attachment_params
-    params.permit(:file, :description, :focus)
+    params.permit(:file, :thumbnail, :description, :focus)
   end
 
   def file_type_error

+ 2 - 2
app/controllers/media_proxy_controller.rb

@@ -28,8 +28,8 @@ class MediaProxyController < ApplicationController
   private
 
   def redownload!
-    @media_attachment.file_remote_url = @media_attachment.remote_url
-    @media_attachment.created_at      = Time.now.utc
+    @media_attachment.download_file!
+    @media_attachment.created_at = Time.now.utc
     @media_attachment.save!
   end
 

+ 4 - 9
app/controllers/settings/pictures_controller.rb

@@ -7,13 +7,8 @@ module Settings
     before_action :set_picture
 
     def destroy
-      if valid_picture
-        account_params = {
-          @picture => nil,
-          (@picture + '_remote_url') => nil,
-        }
-
-        msg = UpdateAccountService.new.call(@account, account_params) ? I18n.t('generic.changes_saved_msg') : nil
+      if valid_picture?
+        msg = I18n.t('generic.changes_saved_msg') if UpdateAccountService.new.call(@account, { @picture => nil, "#{@picture}_remote_url" => '' })
         redirect_to settings_profile_path, notice: msg, status: 303
       else
         bad_request
@@ -30,8 +25,8 @@ module Settings
       @picture = params[:id]
     end
 
-    def valid_picture
-      @picture == 'avatar' || @picture == 'header'
+    def valid_picture?
+      %w(avatar header).include?(@picture)
     end
   end
 end

+ 2 - 1
app/javascript/mastodon/components/status.js

@@ -352,7 +352,8 @@ class Status extends ImmutablePureComponent {
               <Component
                 src={attachment.get('url')}
                 alt={attachment.get('description')}
-                poster={status.getIn(['account', 'avatar_static'])}
+                poster={attachment.get('preview_url') || status.getIn(['account', 'avatar_static'])}
+                blurhash={attachment.get('blurhash')}
                 duration={attachment.getIn(['meta', 'original', 'duration'], 0)}
                 width={this.props.cachedMediaWidth}
                 height={110}

+ 29 - 13
app/javascript/mastodon/features/audio/index.js

@@ -157,6 +157,7 @@ class Audio extends React.PureComponent {
     fullscreen: PropTypes.bool,
     intl: PropTypes.object.isRequired,
     cacheWidth: PropTypes.func,
+    blurhash: PropTypes.string,
   };
 
   state = {
@@ -222,32 +223,42 @@ class Audio extends React.PureComponent {
     window.addEventListener('scroll', this.handleScroll);
     window.addEventListener('resize', this.handleResize, { passive: true });
 
-    const img = new Image();
-    img.crossOrigin = 'anonymous';
-    img.onload = () => this.handlePosterLoad(img);
-    img.src = this.props.poster;
+    if (!this.props.blurhash) {
+      const img = new Image();
+      img.crossOrigin = 'anonymous';
+      img.onload = () => this.handlePosterLoad(img);
+      img.src = this.props.poster;
+    } else {
+      this._setColorScheme();
+      this._decodeBlurhash();
+    }
   }
 
   componentDidUpdate (prevProps, prevState) {
-    if (prevProps.poster !== this.props.poster) {
+    if (prevProps.poster !== this.props.poster && !this.props.blurhash) {
       const img = new Image();
       img.crossOrigin = 'anonymous';
       img.onload = () => this.handlePosterLoad(img);
       img.src = this.props.poster;
     }
 
-    if (prevState.blurhash !== this.state.blurhash) {
-      const context = this.blurhashCanvas.getContext('2d');
-      const pixels = decode(this.state.blurhash, 32, 32);
-      const outputImageData = new ImageData(pixels, 32, 32);
-
-      context.putImageData(outputImageData, 0, 0);
+    if (prevState.blurhash !== this.state.blurhash || prevProps.blurhash !== this.props.blurhash) {
+      this._setColorScheme();
+      this._decodeBlurhash();
     }
 
     this._clear();
     this._draw();
   }
 
+  _decodeBlurhash () {
+    const context = this.blurhashCanvas.getContext('2d');
+    const pixels = decode(this.props.blurhash || this.state.blurhash, 32, 32);
+    const outputImageData = new ImageData(pixels, 32, 32);
+
+    context.putImageData(outputImageData, 0, 0);
+  }
+
   componentWillUnmount () {
     window.removeEventListener('scroll', this.handleScroll);
     window.removeEventListener('resize', this.handleResize);
@@ -415,7 +426,7 @@ class Audio extends React.PureComponent {
   }
 
   handlePosterLoad = image => {
-    const canvas = document.createElement('canvas');
+    const canvas  = document.createElement('canvas');
     const context = canvas.getContext('2d');
 
     canvas.width  = image.width;
@@ -425,10 +436,15 @@ class Audio extends React.PureComponent {
 
     const inputImageData = context.getImageData(0, 0, image.width, image.height);
     const blurhash = encode(inputImageData.data, image.width, image.height, 4, 4);
+
+    this.setState({ blurhash });
+  }
+
+  _setColorScheme () {
+    const blurhash     = this.props.blurhash || this.state.blurhash;
     const averageColor = decodeRGB(decode83(blurhash.slice(2, 6)));
 
     this.setState({
-      blurhash,
       color: adjustColor(averageColor),
       darkText: luma(averageColor) >= 165,
     });

+ 2 - 1
app/javascript/mastodon/features/status/components/detailed_status.js

@@ -125,7 +125,8 @@ class DetailedStatus extends ImmutablePureComponent {
             src={attachment.get('url')}
             alt={attachment.get('description')}
             duration={attachment.getIn(['meta', 'original', 'duration'], 0)}
-            poster={status.getIn(['account', 'avatar_static'])}
+            poster={attachment.get('preview_url') || status.getIn(['account', 'avatar_static'])}
+            blurhash={attachment.get('blurhash')}
             height={150}
           />
         );

+ 10 - 2
app/lib/activitypub/activity/create.rb

@@ -238,12 +238,13 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
 
       begin
         href             = Addressable::URI.parse(attachment['url']).normalize.to_s
-        media_attachment = MediaAttachment.create(account: @account, remote_url: href, description: attachment['summary'].presence || attachment['name'].presence, focus: attachment['focalPoint'], blurhash: supported_blurhash?(attachment['blurhash']) ? attachment['blurhash'] : nil)
+        media_attachment = MediaAttachment.create(account: @account, remote_url: href, thumbnail_remote_url: icon_url_from_attachment(attachment), description: attachment['summary'].presence || attachment['name'].presence, focus: attachment['focalPoint'], blurhash: supported_blurhash?(attachment['blurhash']) ? attachment['blurhash'] : nil)
         media_attachments << media_attachment
 
         next if unsupported_media_type?(attachment['mediaType']) || skip_download?
 
-        media_attachment.file_remote_url = href
+        media_attachment.download_file!
+        media_attachment.download_thumbnail!
         media_attachment.save
       rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError
         RedownloadMediaWorker.perform_in(rand(30..600).seconds, media_attachment.id)
@@ -256,6 +257,13 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
     media_attachments
   end
 
+  def icon_url_from_attachment(attachment)
+    url = attachment['icon'].is_a?(Hash) ? attachment['icon']['url'] : attachment['icon']
+    Addressable::URI.parse(url).normalize.to_s if url.present?
+  rescue Addressable::URI::InvalidURIError
+    nil
+  end
+
   def process_poll
     return unless @object['type'] == 'Question' && (@object['anyOf'].is_a?(Array) || @object['oneOf'].is_a?(Array))
 

+ 14 - 15
app/models/concerns/remotable.rb

@@ -4,12 +4,12 @@ module Remotable
   extend ActiveSupport::Concern
 
   class_methods do
-    def remotable_attachment(attachment_name, limit, suppress_errors: true)
-      attribute_name  = "#{attachment_name}_remote_url".to_sym
-      method_name     = "#{attribute_name}=".to_sym
-      alt_method_name = "reset_#{attachment_name}!".to_sym
+    def remotable_attachment(attachment_name, limit, suppress_errors: true, download_on_assign: true, attribute_name: nil)
+      attribute_name ||= "#{attachment_name}_remote_url".to_sym
+
+      define_method("download_#{attachment_name}!") do
+        url = self[attribute_name]
 
-      define_method method_name do |url|
         return if url.blank?
 
         begin
@@ -18,7 +18,7 @@ module Remotable
           return
         end
 
-        return if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.blank? || (self[attribute_name] == url && send("#{attachment_name}_file_name").present?)
+        return if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.blank?
 
         begin
           Request.new(:get, url).perform do |response|
@@ -36,10 +36,8 @@ module Remotable
 
             basename = SecureRandom.hex(8)
 
-            send("#{attachment_name}_file_name=", basename + extname)
-            send("#{attachment_name}=", StringIO.new(response.body_with_limit(limit)))
-
-            self[attribute_name] = url if has_attribute?(attribute_name)
+            public_send("#{attachment_name}_file_name=", basename + extname)
+            public_send("#{attachment_name}=", StringIO.new(response.body_with_limit(limit)))
           end
         rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError => e
           Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}"
@@ -50,14 +48,15 @@ module Remotable
         end
       end
 
-      define_method alt_method_name do
-        url = self[attribute_name]
+      define_method("#{attribute_name}=") do |url|
+        return if self[attribute_name] == url && public_send("#{attachment_name}_file_name").present?
 
-        return if url.blank?
+        self[attribute_name] = url
 
-        self[attribute_name] = ''
-        send(method_name, url)
+        public_send("download_#{attachment_name}!") if download_on_assign
       end
+
+      alias_method("reset_#{attachment_name}!", "download_#{attachment_name}!")
     end
   end
 

+ 72 - 36
app/models/media_attachment.rb

@@ -21,6 +21,11 @@
 #  blurhash                    :string
 #  processing                  :integer
 #  file_storage_schema_version :integer
+#  thumbnail_file_name         :string
+#  thumbnail_content_type      :string
+#  thumbnail_file_size         :integer
+#  thumbnail_updated_at        :datetime
+#  thumbnail_remote_url        :string
 #
 
 class MediaAttachment < ApplicationRecord
@@ -49,13 +54,13 @@ class MediaAttachment < ApplicationRecord
     original: {
       pixels: 1_638_400, # 1280x1280px
       file_geometry_parser: FastGeometryParser,
-    },
+    }.freeze,
 
     small: {
       pixels: 160_000, # 400x400px
       file_geometry_parser: FastGeometryParser,
       blurhash: BLURHASH_OPTIONS,
-    },
+    }.freeze,
   }.freeze
 
   VIDEO_FORMAT = {
@@ -74,14 +79,14 @@ class MediaAttachment < ApplicationRecord
         'frames:v' => 60 * 60 * 3,
         'crf' => 18,
         'map_metadata' => '-1',
-      },
-    },
+      }.freeze,
+    }.freeze,
   }.freeze
 
   VIDEO_PASSTHROUGH_OPTIONS = {
-    video_codecs: ['h264'],
-    audio_codecs: ['aac', nil],
-    colorspaces: ['yuv420p'],
+    video_codecs: ['h264'].freeze,
+    audio_codecs: ['aac', nil].freeze,
+    colorspaces: ['yuv420p'].freeze,
     options: {
       format: 'mp4',
       convert_options: {
@@ -90,9 +95,9 @@ class MediaAttachment < ApplicationRecord
           'map_metadata' => '-1',
           'c:v' => 'copy',
           'c:a' => 'copy',
-        },
-      },
-    },
+        }.freeze,
+      }.freeze,
+    }.freeze,
   }.freeze
 
   VIDEO_STYLES = {
@@ -101,15 +106,15 @@ class MediaAttachment < ApplicationRecord
         output: {
           'loglevel' => 'fatal',
           vf: 'scale=\'min(400\, iw):min(400\, ih)\':force_original_aspect_ratio=decrease',
-        },
-      },
+        }.freeze,
+      }.freeze,
       format: 'png',
       time: 0,
       file_geometry_parser: FastGeometryParser,
       blurhash: BLURHASH_OPTIONS,
-    },
+    }.freeze,
 
-    original: VIDEO_FORMAT.merge(passthrough_options: VIDEO_PASSTHROUGH_OPTIONS),
+    original: VIDEO_FORMAT.merge(passthrough_options: VIDEO_PASSTHROUGH_OPTIONS).freeze,
   }.freeze
 
   AUDIO_STYLES = {
@@ -119,16 +124,23 @@ class MediaAttachment < ApplicationRecord
       convert_options: {
         output: {
           'loglevel' => 'fatal',
-          'map_metadata' => '-1',
           'q:a' => 2,
-        },
-      },
-    },
+        }.freeze,
+      }.freeze,
+    }.freeze,
   }.freeze
 
   VIDEO_CONVERTED_STYLES = {
-    small: VIDEO_STYLES[:small],
-    original: VIDEO_FORMAT,
+    small: VIDEO_STYLES[:small].freeze,
+    original: VIDEO_FORMAT.freeze,
+  }.freeze
+
+  THUMBNAIL_STYLES = {
+    original: IMAGE_STYLES[:small].freeze,
+  }.freeze
+
+  GLOBAL_CONVERT_OPTIONS = {
+    all: '-quality 90 -strip +set modify-date +set create-date',
   }.freeze
 
   IMAGE_LIMIT = 10.megabytes
@@ -144,18 +156,28 @@ class MediaAttachment < ApplicationRecord
   has_attached_file :file,
                     styles: ->(f) { file_styles f },
                     processors: ->(f) { file_processors f },
-                    convert_options: { all: '-quality 90 -strip +set modify-date +set create-date' }
+                    convert_options: GLOBAL_CONVERT_OPTIONS
 
   validates_attachment_content_type :file, content_type: IMAGE_MIME_TYPES + VIDEO_MIME_TYPES + AUDIO_MIME_TYPES
   validates_attachment_size :file, less_than: IMAGE_LIMIT, unless: :larger_media_format?
   validates_attachment_size :file, less_than: VIDEO_LIMIT, if: :larger_media_format?
-  remotable_attachment :file, VIDEO_LIMIT, suppress_errors: false
+  remotable_attachment :file, VIDEO_LIMIT, suppress_errors: false, download_on_assign: false, attribute_name: :remote_url
+
+  has_attached_file :thumbnail,
+                    styles: THUMBNAIL_STYLES,
+                    processors: [:lazy_thumbnail, :blurhash_transcoder],
+                    convert_options: GLOBAL_CONVERT_OPTIONS
+
+  validates_attachment_content_type :thumbnail, content_type: IMAGE_MIME_TYPES
+  validates_attachment_size :thumbnail, less_than: IMAGE_LIMIT
+  remotable_attachment :thumbnail, IMAGE_LIMIT, suppress_errors: true, download_on_assign: false
 
   include Attachmentable
 
   validates :account, presence: true
   validates :description, length: { maximum: MAX_DESCRIPTION_LENGTH }, if: :local?
   validates :file, presence: true, if: :local?
+  validates :thumbnail, absence: true, if: -> { local? && !audio_or_video? }
 
   scope :attached,   -> { where.not(status_id: nil).or(where.not(scheduled_status_id: nil)) }
   scope :unattached, -> { where(status_id: nil, scheduled_status_id: nil) }
@@ -215,16 +237,21 @@ class MediaAttachment < ApplicationRecord
     @delay_processing
   end
 
+  def delay_processing_for_attachment?(attachment_name)
+    @delay_processing && attachment_name == :file
+  end
+
   after_commit :enqueue_processing, on: :create
   after_commit :reset_parent_cache, on: :update
 
   before_create :prepare_description, unless: :local?
   before_create :set_shortcode
   before_create :set_processing
-  before_create :set_meta
 
-  before_post_process :set_type_and_extension
-  before_post_process :check_video_dimensions
+  after_post_process :set_meta
+
+  before_file_post_process :set_type_and_extension
+  before_file_post_process :check_video_dimensions
 
   class << self
     def supported_mime_types
@@ -237,25 +264,25 @@ class MediaAttachment < ApplicationRecord
 
     private
 
-    def file_styles(f)
-      if f.instance.file_content_type == 'image/gif' || VIDEO_CONVERTIBLE_MIME_TYPES.include?(f.instance.file_content_type)
+    def file_styles(attachment)
+      if attachment.instance.file_content_type == 'image/gif' || VIDEO_CONVERTIBLE_MIME_TYPES.include?(attachment.instance.file_content_type)
         VIDEO_CONVERTED_STYLES
-      elsif IMAGE_MIME_TYPES.include?(f.instance.file_content_type)
+      elsif IMAGE_MIME_TYPES.include?(attachment.instance.file_content_type)
         IMAGE_STYLES
-      elsif VIDEO_MIME_TYPES.include?(f.instance.file_content_type)
+      elsif VIDEO_MIME_TYPES.include?(attachment.instance.file_content_type)
         VIDEO_STYLES
       else
         AUDIO_STYLES
       end
     end
 
-    def file_processors(f)
-      if f.file_content_type == 'image/gif'
+    def file_processors(instance)
+      if instance.file_content_type == 'image/gif'
         [:gif_transcoder, :blurhash_transcoder]
-      elsif VIDEO_MIME_TYPES.include?(f.file_content_type)
+      elsif VIDEO_MIME_TYPES.include?(instance.file_content_type)
         [:video_transcoder, :blurhash_transcoder, :type_corrector]
-      elsif AUDIO_MIME_TYPES.include?(f.file_content_type)
-        [:transcoder, :type_corrector]
+      elsif AUDIO_MIME_TYPES.include?(instance.file_content_type)
+        [:image_extractor, :transcoder, :type_corrector]
       else
         [:lazy_thumbnail, :blurhash_transcoder, :type_corrector]
       end
@@ -298,7 +325,7 @@ class MediaAttachment < ApplicationRecord
   def check_video_dimensions
     return unless (video? || gifv?) && file.queued_for_write[:original].present?
 
-    movie = FFMPEG::Movie.new(file.queued_for_write[:original].path)
+    movie = ffmpeg_data(file.queued_for_write[:original].path)
 
     return unless movie.valid?
 
@@ -317,6 +344,8 @@ class MediaAttachment < ApplicationRecord
       meta[style] = style == :small || image? ? image_geometry(file) : video_metadata(file)
     end
 
+    meta[:small] = image_geometry(thumbnail.queued_for_write[:original]) if thumbnail.queued_for_write.key?(:original)
+
     meta
   end
 
@@ -334,7 +363,7 @@ class MediaAttachment < ApplicationRecord
   end
 
   def video_metadata(file)
-    movie = FFMPEG::Movie.new(file.path)
+    movie = ffmpeg_data(file.path)
 
     return {} unless movie.valid?
 
@@ -347,6 +376,13 @@ class MediaAttachment < ApplicationRecord
     }.compact
   end
 
+  # We call this method about 3 different times on potentially different
+  # paths but ultimately the same file, so it makes sense to memoize the
+  # result while disregarding the path
+  def ffmpeg_data(path = nil)
+    @ffmpeg_data ||= FFMPEG::Movie.new(path)
+  end
+
   def enqueue_processing
     PostProcessMediaWorker.perform_async(id) if delay_processing?
   end

+ 10 - 0
app/serializers/activitypub/note_serializer.rb

@@ -167,6 +167,8 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer
     attributes :type, :media_type, :url, :name, :blurhash
     attribute :focal_point, if: :focal_point?
 
+    has_one :icon, serializer: ActivityPub::ImageSerializer, if: :thumbnail?
+
     def type
       'Document'
     end
@@ -190,6 +192,14 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer
     def focal_point
       [object.file.meta['focus']['x'], object.file.meta['focus']['y']]
     end
+
+    def icon
+      object.thumbnail
+    end
+
+    def thumbnail?
+      object.thumbnail.present?
+    end
   end
 
   class MentionSerializer < ActivityPub::Serializer

+ 3 - 1
app/serializers/rest/media_attachment_serializer.rb

@@ -28,7 +28,9 @@ class REST::MediaAttachmentSerializer < ActiveModel::Serializer
   def preview_url
     if object.needs_redownload?
       media_proxy_url(object.id, :small)
-    else
+    elsif object.thumbnail.present?
+      full_asset_url(object.thumbnail.url(:original))
+    elsif object.file.styles.key?(:small)
       full_asset_url(object.file.url(:small))
     end
   end

+ 2 - 2
app/services/activitypub/process_account_service.rb

@@ -89,8 +89,8 @@ class ActivityPub::ProcessAccountService < BaseService
   end
 
   def set_fetchable_attributes!
-    @account.avatar_remote_url = image_url('icon')  unless skip_download?
-    @account.header_remote_url = image_url('image') unless skip_download?
+    @account.avatar_remote_url = image_url('icon')  || '' unless skip_download?
+    @account.header_remote_url = image_url('image') || '' unless skip_download?
     @account.public_key        = public_key || ''
     @account.statuses_count    = outbox_total_items    if outbox_total_items.present?
     @account.following_count   = following_total_items if following_total_items.present?

+ 1 - 1
app/views/statuses/_detailed_status.html.haml

@@ -33,7 +33,7 @@
         = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments }
     - elsif status.media_attachments.first.audio?
       - audio = status.media_attachments.first
-      = react_component :audio, src: audio.file.url(:original), poster: full_asset_url(status.account.avatar_static_url), width: 670, height: 380, alt: audio.description, duration: audio.file.meta.dig(:original, :duration) do
+      = react_component :audio, src: audio.file.url(:original), poster: audio.thumbnail.present? ? audio.thumbnail.url : status.account.avatar_static_url, blurhash: audio.blurhash, width: 670, height: 380, alt: audio.description, duration: audio.file.meta.dig('original', 'duration') do
         = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments }
     - else
       = react_component :media_gallery, height: 380, sensitive: status.sensitive?, standalone: true, autoplay: autoplay, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } do

+ 1 - 1
app/views/statuses/_simple_status.html.haml

@@ -39,7 +39,7 @@
         = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments }
     - elsif status.media_attachments.first.audio?
       - audio = status.media_attachments.first
-      = react_component :audio, src: audio.file.url(:original), poster: full_asset_url(status.account.avatar_static_url), width: 610, height: 343, alt: audio.description, duration: audio.file.meta.dig(:original, :duration) do
+      = react_component :audio, src: audio.file.url(:original), poster: audio.thumbnail.present? ? audio.thumbnail.url : status.account.avatar_static_url, blurhash: audio.blurhash, width: 610, height: 343, alt: audio.description, duration: audio.file.meta.dig('original', 'duration') do
         = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments }
     - else
       = react_component :media_gallery, height: 343, sensitive: status.sensitive?, autoplay: autoplay, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } do

+ 1 - 1
app/workers/post_process_media_worker.rb

@@ -32,7 +32,7 @@ class PostProcessMediaWorker
 
     media_attachment.file.reprocess!(:original)
     media_attachment.processing = :complete
-    media_attachment.file_meta = previous_meta
+    media_attachment.file_meta = previous_meta.merge(media_attachment.file_meta).with_indifferent_access.slice(:focus, :original, :small)
     media_attachment.save
   rescue ActiveRecord::RecordNotFound
     true

+ 2 - 1
app/workers/redownload_media_worker.rb

@@ -11,7 +11,8 @@ class RedownloadMediaWorker
 
     return if media_attachment.remote_url.blank?
 
-    media_attachment.file_remote_url = media_attachment.remote_url
+    media_attachment.download_file!
+    media_attachment.download_thumbnail!
     media_attachment.save
   rescue ActiveRecord::RecordNotFound
     true

+ 11 - 0
db/migrate/20200627125810_add_thumbnail_columns_to_media_attachments.rb

@@ -0,0 +1,11 @@
+class AddThumbnailColumnsToMediaAttachments < ActiveRecord::Migration[5.2]
+  def up
+    add_attachment :media_attachments, :thumbnail
+    add_column :media_attachments, :thumbnail_remote_url, :string
+  end
+
+  def down
+    remove_attachment :media_attachments, :thumbnail
+    remove_column :media_attachments, :thumbnail_remote_url
+  end
+end

+ 6 - 1
db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_06_20_164023) do
+ActiveRecord::Schema.define(version: 2020_06_27_125810) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -489,6 +489,11 @@ ActiveRecord::Schema.define(version: 2020_06_20_164023) do
     t.string "blurhash"
     t.integer "processing"
     t.integer "file_storage_schema_version"
+    t.string "thumbnail_file_name"
+    t.string "thumbnail_content_type"
+    t.integer "thumbnail_file_size"
+    t.datetime "thumbnail_updated_at"
+    t.string "thumbnail_remote_url"
     t.index ["account_id"], name: "index_media_attachments_on_account_id"
     t.index ["scheduled_status_id"], name: "index_media_attachments_on_scheduled_status_id"
     t.index ["shortcode"], name: "index_media_attachments_on_shortcode", unique: true

+ 6 - 4
lib/mastodon/media_cli.rb

@@ -31,10 +31,11 @@ module Mastodon
       processed, aggregate = parallelize_with_progress(MediaAttachment.cached.where.not(remote_url: '').where('created_at < ?', time_ago)) do |media_attachment|
         next if media_attachment.file.blank?
 
-        size = media_attachment.file_file_size
+        size = media_attachment.file_file_size + (media_attachment.thumbnail_file_size || 0)
 
         unless options[:dry_run]
           media_attachment.file.destroy
+          media_attachment.thumbnail.destroy
           media_attachment.save
         end
 
@@ -227,11 +228,12 @@ module Mastodon
         next if media_attachment.remote_url.blank? || (!options[:force] && media_attachment.file_file_name.present?)
 
         unless options[:dry_run]
-          media_attachment.file_remote_url = media_attachment.remote_url
+          media_attachment.reset_file!
+          media_attachment.reset_thumbnail!
           media_attachment.save
         end
 
-        media_attachment.file_file_size
+        media_attachment.file_file_size + (media_attachment.thumbnail_file_size || 0)
       end
 
       say("Downloaded #{processed} media attachments (approx. #{number_to_human_size(aggregate)})#{dry_run}", :green, true)
@@ -239,7 +241,7 @@ module Mastodon
 
     desc 'usage', 'Calculate disk space consumed by Mastodon'
     def usage
-      say("Attachments:\t#{number_to_human_size(MediaAttachment.sum(:file_file_size))} (#{number_to_human_size(MediaAttachment.where(account: Account.local).sum(:file_file_size))} local)")
+      say("Attachments:\t#{number_to_human_size(MediaAttachment.sum(Arel.sql('COALESCE(file_file_size, 0) + COALESCE(thumbnail_file_size, 0)')))} (#{number_to_human_size(MediaAttachment.where(account: Account.local).sum(Arel.sql('COALESCE(file_file_size, 0) + COALESCE(thumbnail_file_size, 0)')))} local)")
       say("Custom emoji:\t#{number_to_human_size(CustomEmoji.sum(:image_file_size))} (#{number_to_human_size(CustomEmoji.local.sum(:image_file_size))} local)")
       say("Preview cards:\t#{number_to_human_size(PreviewCard.sum(:image_file_size))}")
       say("Avatars:\t#{number_to_human_size(Account.sum(:avatar_file_size))} (#{number_to_human_size(Account.local.sum(:avatar_file_size))} local)")

+ 1 - 1
lib/paperclip/attachment_extensions.rb

@@ -7,7 +7,7 @@ module Paperclip
     # usage, and we still want to generate thumbnails straight
     # away, it's the only style we need to exclude
     def process_style?(style_name, style_args)
-      if style_name == :original && instance.respond_to?(:delay_processing?) && instance.delay_processing?
+      if style_name == :original && instance.respond_to?(:delay_processing_for_attachment?) && instance.delay_processing_for_attachment?(name)
         false
       else
         style_args.empty? || style_args.include?(style_name)

+ 49 - 0
lib/paperclip/image_extractor.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'mime/types/columnar'
+
+module Paperclip
+  class ImageExtractor < Paperclip::Processor
+    IMAGE_EXTRACTION_OPTIONS = {
+      convert_options: {
+        output: {
+          'loglevel' => 'fatal',
+          vf: 'scale=\'min(400\, iw):min(400\, ih)\':force_original_aspect_ratio=decrease',
+        }.freeze,
+      }.freeze,
+      format: 'png',
+      time: -1,
+      file_geometry_parser: FastGeometryParser,
+    }.freeze
+
+    def make
+      return @file unless options[:style] == :original
+
+      image = begin
+        begin
+          Paperclip::Transcoder.make(file, IMAGE_EXTRACTION_OPTIONS.dup, attachment)
+        rescue Paperclip::Error, ::Av::CommandError
+          nil
+        end
+      end
+
+      unless image.nil?
+        begin
+          attachment.instance.thumbnail = image if image.size.positive?
+        ensure
+          # Paperclip does not automatically delete the source file of
+          # a new attachment while working on copies of it, so we need
+          # to make sure it's cleaned up
+
+          begin
+            FileUtils.rm(image)
+          rescue Errno::ENOENT
+            nil
+          end
+        end
+      end
+
+      @file
+    end
+  end
+end

+ 6 - 4
lib/paperclip/type_corrector.rb

@@ -5,13 +5,15 @@ require 'mime/types/columnar'
 module Paperclip
   class TypeCorrector < Paperclip::Processor
     def make
-      target_extension = options[:format]
-      extension        = File.extname(attachment.instance.file_file_name)
+      return @file unless options[:format]
+
+      target_extension = '.' + options[:format]
+      extension        = File.extname(attachment.instance_read(:file_name))
 
       return @file unless options[:style] == :original && target_extension && extension != target_extension
 
-      attachment.instance.file_content_type = options[:content_type] || attachment.instance.file_content_type
-      attachment.instance.file_file_name    = File.basename(attachment.instance.file_file_name, '.*') + '.' + target_extension
+      attachment.instance_write(:content_type, options[:content_type] || attachment.instance_read(:content_type))
+      attachment.instance_write(:file_name, File.basename(attachment.instance_read(:file_name), '.*') + target_extension)
 
       @file
     end

+ 12 - 41
spec/models/concerns/remotable_spec.rb

@@ -58,7 +58,11 @@ RSpec.describe Remotable do
       expect(foo).to respond_to(:reset_hoge!)
     end
 
-    describe '#hoge_remote_url' do
+    it 'defines a method #download_hoge!' do
+      expect(foo).to respond_to(:download_hoge!)
+    end
+
+    describe '#hoge_remote_url=' do
       before do
         request
       end
@@ -138,8 +142,8 @@ RSpec.describe Remotable do
           let(:code) { 500 }
 
           it 'calls not send' do
-            expect(foo).not_to receive(:send).with("#{hoge}=", any_args)
-            expect(foo).not_to receive(:send).with("#{hoge}_file_name=", any_args)
+            expect(foo).not_to receive(:public_send).with("#{hoge}=", any_args)
+            expect(foo).not_to receive(:public_send).with("#{hoge}_file_name=", any_args)
             foo.hoge_remote_url = url
           end
         end
@@ -159,26 +163,14 @@ RSpec.describe Remotable do
               allow(SecureRandom).to receive(:hex).and_return(basename)
               allow(StringIO).to receive(:new).with(anything).and_return(string_io)
 
-              expect(foo).to receive(:send).with("#{hoge}=", string_io)
-              expect(foo).to receive(:send).with("#{hoge}_file_name=", basename + extname)
-              foo.hoge_remote_url = url
-            end
-          end
+              expect(foo).to receive(:public_send).with("download_#{hoge}!")
 
-          context 'if has_attribute?' do
-            it 'calls foo[attribute_name] = url' do
-              allow(foo).to receive(:has_attribute?).with(attribute_name).and_return(true)
-              expect(foo).to receive('[]=').with(attribute_name, url)
               foo.hoge_remote_url = url
-            end
-          end
 
-          context 'unless has_attribute?' do
-            it 'calls not foo[attribute_name] = url' do
-              allow(foo).to receive(:has_attribute?)
-                .with(attribute_name).and_return(false)
-              expect(foo).not_to receive('[]=').with(attribute_name, url)
-              foo.hoge_remote_url = url
+              expect(foo).to receive(:public_send).with("#{hoge}=", string_io)
+              expect(foo).to receive(:public_send).with("#{hoge}_file_name=", basename + extname)
+
+              foo.download_hoge!
             end
           end
         end
@@ -205,26 +197,5 @@ RSpec.describe Remotable do
         end
       end
     end
-
-    describe '#reset_hoge!' do
-      context 'if url.blank?' do
-        it 'returns nil, without clearing foo[attribute_name] and calling #hoge_remote_url=' do
-          url = nil
-          expect(foo).not_to receive(:send).with(:hoge_remote_url=, url)
-          foo[attribute_name] = url
-          expect(foo.reset_hoge!).to be_nil
-          expect(foo[attribute_name]).to be_nil
-        end
-      end
-
-      context 'unless url.blank?' do
-        it 'clears foo[attribute_name] and calls #hoge_remote_url=' do
-          foo[attribute_name] = url
-          expect(foo).to receive(:send).with(:hoge_remote_url=, url)
-          foo.reset_hoge!
-          expect(foo[attribute_name]).to be ''
-        end
-      end
-    end
   end
 end