Bound the work of previewing an attachment

A video's preview and a picture's thumbnail are made inside the request that posts the message, and
nothing bounded how long either could take.

- The video preview filter also selects any frame from 5 seconds on. Rails' filter takes the second
  frame it selects, which a video with a single keyframe and no scene change only gives at its end, so
  ffmpeg decoded all of it.
- TimeLimitedVideoPreviewer gives ffmpeg 10 seconds of wall-clock time, kills it past that, and reports
  a failed preview, so the message is posted without one.
- Pictures and videos above 250 megapixels, or whose size couldn't be read, get no preview: decoding
  costs in proportion to the pixels, however small the file.
- The view shows a preview only if it was made when the message was posted. Its URL used to make it on
  view, so a preview that failed or was skipped would be attempted again on every view. The cached
  presentation's version goes up, so cached messages pick this up.
- A video's poster is made, when the message is posted, at the size the view shows it. The full-size WebP
  made until now wasn't shown anywhere, and encoding it costs in proportion to the frame's pixels.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa

app/helpers/messages/attachment_presentation.rb

@@ -17,11 +17,16 @@ class Messages::AttachmentPresentation
     attr_reader :message, :context
     delegate :tag, :link_to, :broadcast_image_tag, :rails_blob_path, :url_for, to: :context
 
+    # Previews are made when the message is posted, within limits. One that wasn't, because the file was too large or
+    # the preview failed, isn't attempted again from here: its URL would make it on every view.
     def render_preview
-      if message.attachment.video?
+      case
+      when message.attachment.video?
         video_preview_tag
-      else
+      when message.attachment.representation(:thumb).processed?
         lightboxed_image_preview_tag
+      else
+        render_link
       end
     end
 
@@ -30,11 +35,16 @@ class Messages::AttachmentPresentation
 
       inline_media_dimension_constraints(width, height) do
         tag.video \
-          src: rails_blob_path(message.attachment), poster: url_for(message.attachment.preview(format: :webp, resize_to_limit: [ Message::THUMBNAIL_MAX_WIDTH, Message::THUMBNAIL_MAX_HEIGHT ])),
+          src: rails_blob_path(message.attachment), poster: video_poster_url,
           controls: true, preload: :none, width: "100%", height: "100%", class: "message__attachment"
       end
     end
 
+    def video_poster_url
+      poster = message.attachment.preview(:poster)
+      url_for(poster) if poster.processed?
+    end
+
     def lightboxed_image_preview_tag
       width, height = preview_dimensions
 

app/models/message/attachment.rb

@@ -4,9 +4,14 @@ module Message::Attachment
   THUMBNAIL_MAX_WIDTH = 1200
   THUMBNAIL_MAX_HEIGHT = 800
 
+  # Decoding a picture, or a video's frame, costs in proportion to its pixels, however small the file. The largest
+  # phone photos have 200 million.
+  THUMBNAIL_MAX_PIXELS = 250_000_000
+
   included do
     has_one_attached :attachment do |attachable|
       attachable.variant :thumb, resize_to_limit: [ THUMBNAIL_MAX_WIDTH, THUMBNAIL_MAX_HEIGHT ]
+      attachable.variant :poster, format: :webp, resize_to_limit: [ THUMBNAIL_MAX_WIDTH, THUMBNAIL_MAX_HEIGHT ]
     end
   end
 
@@ -32,13 +37,23 @@ module Message::Attachment
 
     # A file that ffmpeg or libvips can't decode is still the message: post it without a preview.
     def process_attachment_thumbnail
+      return if too_many_pixels_to_preview?
+
       case
       when attachment.video?
-        attachment.preview(format: :webp).processed
+        attachment.preview(:poster).processed
       when attachment.representable?
         attachment.representation(:thumb).processed
       end
     rescue ActiveStorage::PreviewError, Vips::Error => error
       Rails.logger.warn "Posted #{attachment.filename} without a preview: #{error.class}: #{error.message.lines.first&.chomp}"
     end
+
+    # Without a size, the analyzer couldn't read the file's header, and the previewer wouldn't either.
+    def too_many_pixels_to_preview?
+      if attachment.image? || attachment.video?
+        width, height = attachment.metadata.values_at(:width, :height)
+        width.nil? || height.nil? || width * height > THUMBNAIL_MAX_PIXELS
+      end
+    end
 end

app/views/messages/_message.html.erb

@@ -1,7 +1,7 @@
 <%# Be sure to check/update messages/_template.html.erb when changing this file %>
 
 <%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %>
-<% cache [ message, "presentation-v3" ] do %>
+<% cache [ message, "presentation-v4" ] do %>
   <%= message_tag message do %>
     <h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2>
 

config/initializers/active_storage.rb

@@ -1,5 +1,20 @@
+require "rails_ext/time_limited_video_previewer"
+
 ActiveSupport.on_load(:active_storage_blob) do
   ActiveStorage::DiskController.after_action only: :show do
     response.set_header("Cache-Control", "max-age=3600, public")
   end
 end
+
+Rails.application.configure do
+  # Rails' filter takes the second of the frames it selects (the first one, keyframes, scene changes), so a video
+  # with a single keyframe and no scene change is decoded to its end. Selecting any frame from 5 seconds on stops
+  # it there.
+  config.active_storage.video_preview_arguments =
+    "-vf 'select=eq(n\\,0)+eq(key\\,1)+gt(scene\\,0.015)+gte(t\\,5),loop=loop=-1:size=2,trim=start_frame=1'" \
+    " -frames:v 1 -f image2"
+
+  config.active_storage.previewers = config.active_storage.previewers.map do |previewer|
+    previewer == ActiveStorage::Previewer::VideoPreviewer ? TimeLimitedVideoPreviewer : previewer
+  end
+end

lib/rails_ext/time_limited_video_previewer.rb

@@ -0,0 +1,27 @@
+# Rails waits for ffmpeg however long it takes, and a video's preview is drawn inside the request that posts it.
+# This previewer gives ffmpeg a wall-clock limit, kills it past that, and reports a failed preview, so the message
+# is posted without one.
+class TimeLimitedVideoPreviewer < ActiveStorage::Previewer::VideoPreviewer
+  TIME_LIMIT = 10 # seconds
+
+  private
+    def capture(*argv, to:)
+      to.binmode
+
+      open_tempfile do |err|
+        IO.popen(argv, in: IO::NULL, err: err) do |out|
+          Timeout.timeout(TIME_LIMIT) { IO.copy_stream(out, to) }
+        rescue Timeout::Error
+          Process.kill :KILL, out.pid
+          raise ActiveStorage::PreviewError, "#{argv.first} took longer than #{TIME_LIMIT} seconds"
+        end
+        err.rewind
+
+        unless $?.success?
+          raise ActiveStorage::PreviewError, "#{argv.first} failed (status #{$?.exitstatus}): #{err.read.to_s.chomp}"
+        end
+      end
+
+      to.rewind
+    end
+end

test/controllers/messages_caching_test.rb

@@ -27,6 +27,22 @@ class MessagesCachingTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "a page of messages loads whether their previews were made along with the messages, not one at a time" do
+    room = rooms(:watercooler)
+    2.times do |copy|
+      { "moon.jpg" => "image/jpeg", "alpha-centuri.mov" => "video/quicktime" }.each do |file, content_type|
+        room.messages.create_with_attachment! creator: users(:david), client_message_id: "#{copy}-#{file}", attachment: fixture_file_upload(file, content_type)
+      end
+    end
+
+    assert_no_queries_match(/"active_storage_\w+"\."(id|blob_id|record_id)" = \?/) do
+      get room_messages_url(room)
+    end
+    assert_response :success
+    assert_select "img[src*='moon.jpg']", count: 2
+    assert_select "video[poster]", count: 2
+  end
+
   private
     def with_memory_cache
       old_cache = Rails.cache

test/helpers/messages_helper_test.rb

@@ -24,4 +24,41 @@ class MessagesHelperTest < ActionView::TestCase
     assert_match /<a href="https:\/\/example\.com"[^>]*>example<\/a>/, presentation
     assert_match /<strong>bold<\/strong>/, presentation
   end
+
+  test "message_presentation shows an image's thumbnail made when it was posted" do
+    presentation = view.message_presentation(attachment_message("moon.jpg", "image/jpeg", processed: true))
+
+    assert_match %r{<img[^>]+src="[^"]*/representations/[^"]*moon\.jpg"}, presentation
+  end
+
+  test "message_presentation links an image whose thumbnail wasn't made, rather than making it on view" do
+    presentation = view.message_presentation(attachment_message("moon.jpg", "image/jpeg", processed: false))
+
+    assert_no_match %r{/representations/}, presentation
+    assert_match %r{<span>moon\.jpg</span>}, presentation
+  end
+
+  test "message_presentation gives a video the poster made when it was posted" do
+    presentation = view.message_presentation(attachment_message("alpha-centuri.mov", "video/quicktime", processed: true))
+
+    assert_match %r{<video[^>]+poster="[^"]*/representations/[^"]*alpha-centuri}, presentation
+  end
+
+  test "message_presentation shows a video whose poster wasn't made without one, rather than making it on view" do
+    presentation = view.message_presentation(attachment_message("alpha-centuri.mov", "video/quicktime", processed: false))
+
+    assert_match %r{<video[^>]+src="[^"]*alpha-centuri\.mov"}, presentation
+    assert_no_match %r{poster=|/representations/}, presentation
+  end
+
+  private
+    def attachment_message(file, content_type, processed:)
+      attributes = { creator: users(:jason), client_message_id: "0015", attachment: fixture_file_upload(file, content_type) }
+
+      if processed
+        rooms(:pets).messages.create_with_attachment!(attributes)
+      else
+        rooms(:pets).messages.create!(attributes)
+      end
+    end
 end

test/models/message/attachment_test.rb

@@ -34,7 +34,68 @@ class Message::AttachmentTest < ActiveSupport::TestCase
     assert_not message.attachment.preview(format: :webp).image.attached?
   end
 
+  test "creating a message makes the video poster that the room shows" do
+    message = create_attachment_message("alpha-centuri.mov", "video/quicktime")
+
+    assert_no_difference -> { ActiveStorage::VariantRecord.count } do
+      message.reload.attachment.preview(:poster).processed
+    end
+  end
+
+  test "video previews are drawn by the previewer with a time limit, from a frame within the first 5 seconds" do
+    assert_includes ActiveStorage.previewers, TimeLimitedVideoPreviewer
+    assert_not_includes ActiveStorage.previewers, ActiveStorage::Previewer::VideoPreviewer
+    assert_includes ActiveStorage.video_preview_arguments, "gte(t\\,5)"
+  end
+
+  test "creating a message gives up on a video preview that takes longer than the time limit" do
+    started = nil
+    message = nil
+
+    with_ffmpeg_sleeping(5.seconds) do
+      stub_const(TimeLimitedVideoPreviewer, :TIME_LIMIT, 0.2) do
+        started = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+        message = create_attachment_message("alpha-centuri.mov", "video/quicktime")
+      end
+    end
+
+    assert_operator Process.clock_gettime(Process::CLOCK_MONOTONIC) - started, :<, 2
+    assert_not message.reload.attachment.preview(format: :webp).image.attached?
+  end
+
+  test "creating a message makes no thumbnail of an image with more pixels than the limit" do
+    moon = Vips::Image.new_from_file(file_fixture("moon.jpg").to_s)
+
+    stub_const(Message::Attachment, :THUMBNAIL_MAX_PIXELS, moon.width * moon.height) do
+      assert create_attachment_message("moon.jpg", "image/jpeg").attachment.representation(:thumb).processed?
+    end
+
+    stub_const(Message::Attachment, :THUMBNAIL_MAX_PIXELS, moon.width * moon.height - 1) do
+      assert_not create_attachment_message("moon.jpg", "image/jpeg").attachment.representation(:thumb).processed?
+    end
+  end
+
+  test "creating a message makes no preview of a video with more pixels than the limit" do
+    stub_const(Message::Attachment, :THUMBNAIL_MAX_PIXELS, 1) do
+      message = create_attachment_message("alpha-centuri.mov", "video/quicktime")
+      assert_not message.reload.attachment.preview(format: :webp).image.attached?
+    end
+  end
+
   private
+    def with_ffmpeg_sleeping(duration)
+      Tempfile.create("ffmpeg") do |ffmpeg|
+        ffmpeg.write "#!/bin/sh\n[ \"$1\" = \"-version\" ] && exit 0\nexec sleep #{duration.to_i}\n"
+        ffmpeg.close
+        File.chmod 0o755, ffmpeg.path
+
+        previous, ActiveStorage.paths[:ffmpeg] = ActiveStorage.paths[:ffmpeg], ffmpeg.path
+        yield
+      ensure
+        ActiveStorage.paths[:ffmpeg] = previous
+      end
+    end
+
     def create_attachment_message(file, content_type)
       rooms(:hq).messages.create_with_attachment! \
         creator: users(:david),