Switched thumbnail downloading to use yt-dlp (#281)

lib/pinchflat/downloading/media_downloader.ex

@@ -84,7 +84,7 @@ defmodule Pinchflat.Downloading.MediaDownloader do
           # IDEA: might be worth kicking off a job for this since thumbnail fetching
           # could fail and I want to handle that in isolation
           metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_with_preloads, parsed_json),
-          thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_with_preloads, parsed_json)
+          thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_with_preloads)
         }
       })
 

lib/pinchflat/metadata/metadata_file_helpers.ex

@@ -11,6 +11,8 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do
 
   alias Pinchflat.Utils.FilesystemUtils
 
+  alias Pinchflat.YtDlp.Media, as: YtDlpMedia
+
   @doc """
   Returns the directory where metadata for a database record should be stored.
 
@@ -54,35 +56,19 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do
 
   @doc """
   Downloads and stores a thumbnail for a media item, returning the filepath.
-  Chooses the highest quality thumbnail available (preferring jpg). Returns
-  nil if no thumbnails are available.
+  Chooses the highest quality thumbnail available and converts it to a JPG
+
+  Returns nil if no thumbnail is available or if yt-dlp encounters an error
 
   Returns binary() | nil
   """
-  def download_and_store_thumbnail_for(database_record, metadata_map) do
-    thumbnails =
-      (metadata_map["thumbnails"] || [])
-      # Give it a low preference if the `preference` key doesn't exist
-      |> Enum.map(&Map.put_new(&1, "preference", -1000))
-      # Give it a low preference if image isn't a jpg
-      |> Enum.map(fn t ->
-        preference_weight = if String.ends_with?(t["url"], ".jpg"), do: t["preference"], else: t["preference"] - 1000
+  def download_and_store_thumbnail_for(database_record) do
+    yt_dlp_filepath = generate_filepath_for(database_record, "thumbnail.%(ext)s")
+    real_filepath = generate_filepath_for(database_record, "thumbnail.jpg")
 
-        Map.put(t, "preference", preference_weight)
-      end)
-
-    case Enum.sort_by(thumbnails, & &1["preference"], :desc) do
-      [thumbnail_map | _] ->
-        thumbnail_url = thumbnail_map["url"]
-        filepath = generate_filepath_for(database_record, Path.basename(thumbnail_url))
-        thumbnail_blob = fetch_thumbnail_from_url(thumbnail_url)
-
-        :ok = FilesystemUtils.write_p!(filepath, thumbnail_blob)
-
-        filepath
-
-      _ ->
-        nil
+    case YtDlpMedia.download_thumbnail(database_record.original_url, output: yt_dlp_filepath) do
+      {:ok, _} -> real_filepath
+      _ -> nil
     end
   end
 
@@ -138,13 +124,6 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do
     end
   end
 
-  defp fetch_thumbnail_from_url(url) do
-    http_client = Application.get_env(:pinchflat, :http_client, Pinchflat.HTTP.HTTPClient)
-    {:ok, body} = http_client.get(url, [], body_format: :binary)
-
-    body
-  end
-
   defp generate_filepath_for(database_record, filename) do
     Path.join([
       metadata_directory_for(database_record),

lib/pinchflat/yt_dlp/media.ex

@@ -46,6 +46,20 @@ defmodule Pinchflat.YtDlp.Media do
     end
   end
 
+  @doc """
+  Downloads a thumbnail for a single piece of media. Usually used for
+  downloading thumbnails for internal use
+
+  Returns {:ok, ""} | {:error, any, ...}.
+  """
+  def download_thumbnail(url, command_opts \\ []) do
+    opts = [:no_simulate, :skip_download, :write_thumbnail, convert_thumbnail: "jpg"] ++ command_opts
+
+    # NOTE: it doesn't seem like this command actually returns anything in `after_move` since
+    # we aren't downloading the main media file
+    backend_runner().run(url, opts, "after_move:%()j")
+  end
+
   @doc """
   Returns a map representing the media at the given URL.
 

test/pinchflat/downloading/media_download_worker_test.exs

@@ -9,6 +9,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
   alias Pinchflat.Downloading.MediaDownloadWorker
 
   setup do
+    stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
     stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end)
     stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
 

test/pinchflat/downloading/media_downloader_test.exs

@@ -14,9 +14,8 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
         [:metadata, source: :media_profile]
       )
 
-    stub(HTTPClientMock, :get, fn _url, _headers, _opts ->
-      {:ok, ""}
-    end)
+    stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
+    stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
 
     {:ok, %{media_item: media_item}}
   end
@@ -44,7 +43,7 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
       assert {:ok, updated_media_item} = MediaDownloader.download_for_media_item(media_item)
 
       assert updated_media_item.metadata.metadata_filepath =~ "media_items/#{media_item.id}/metadata.json.gz"
-      assert updated_media_item.metadata.thumbnail_filepath =~ "media_items/#{media_item.id}/maxresdefault.jpg"
+      assert updated_media_item.metadata.thumbnail_filepath =~ "media_items/#{media_item.id}/thumbnail.jpg"
     end
 
     test "non-recoverable errors are passed through", %{media_item: media_item} do

test/pinchflat/media_test.exs

@@ -740,14 +740,13 @@ defmodule Pinchflat.MediaTest do
     end
 
     test "does delete the media item's metadata files" do
-      stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
       media_item = Repo.preload(media_item_with_attachments(), :metadata)
 
       update_attrs = %{
         metadata: %{
           metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, %{}),
-          thumbnail_filepath:
-            MetadataFileHelpers.download_and_store_thumbnail_for(media_item, render_parsed_metadata(:media_metadata))
+          thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_item)
         }
       }
 
@@ -773,14 +772,13 @@ defmodule Pinchflat.MediaTest do
     end
 
     test "deletes the media item's metadata files" do
-      stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
       media_item = Repo.preload(media_item_with_attachments(), :metadata)
 
       update_attrs = %{
         metadata: %{
           metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, %{}),
-          thumbnail_filepath:
-            MetadataFileHelpers.download_and_store_thumbnail_for(media_item, render_parsed_metadata(:media_metadata))
+          thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_item)
         }
       }
 
@@ -860,14 +858,13 @@ defmodule Pinchflat.MediaTest do
     end
 
     test "does not delete the media item's metadata files" do
-      stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
       media_item = Repo.preload(media_item_with_attachments(), :metadata)
 
       update_attrs = %{
         metadata: %{
           metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, %{}),
-          thumbnail_filepath:
-            MetadataFileHelpers.download_and_store_thumbnail_for(media_item, render_parsed_metadata(:media_metadata))
+          thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_item)
         }
       }
 

test/pinchflat/metadata/metadata_file_helpers_test.exs

@@ -50,82 +50,37 @@ defmodule Pinchflat.Metadata.MetadataFileHelpersTest do
   end
 
   describe "download_and_store_thumbnail_for/2" do
-    setup do
-      # This tests that the HTTP endpoint is being called with every test
-      expect(HTTPClientMock, :get, fn _url, _headers, _opts ->
-        {:ok, "thumbnail data"}
+    test "returns the filepath", %{media_item: media_item} do
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
+
+      filepath = Helpers.download_and_store_thumbnail_for(media_item)
+
+      assert filepath =~ ~r{/media_items/#{media_item.id}/thumbnail.jpg}
+    end
+
+    test "calls yt-dlp with the expected options", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn url, opts, ot ->
+        assert url == media_item.original_url
+        assert ot == "after_move:%()j"
+
+        assert opts == [
+                 :no_simulate,
+                 :skip_download,
+                 :write_thumbnail,
+                 convert_thumbnail: "jpg",
+                 output: "/tmp/test/metadata/media_items/1/thumbnail.%(ext)s"
+               ]
+
+        {:ok, ""}
       end)
 
-      metadata = render_parsed_metadata(:media_metadata)
-
-      {:ok, %{metadata: metadata}}
+      Helpers.download_and_store_thumbnail_for(media_item)
     end
 
-    test "returns the filepath", %{media_item: media_item, metadata: metadata} do
-      filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata)
+    test "returns nil if yt-dlp fails", %{media_item: media_item} do
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "error"} end)
 
-      assert filepath =~ ~r{/media_items/#{media_item.id}/maxresdefault.jpg}
-    end
-
-    test "creates folder structure based on passed record", %{media_item: media_item, metadata: metadata} do
-      filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata)
-
-      assert File.exists?(Path.dirname(filepath))
-    end
-
-    test "chooses the highest preference jpg thumbnail available", %{media_item: media_item} do
-      metadata = %{
-        "thumbnails" => [
-          %{"url" => "https://i.ytimg.com/vi/ABC123/img_1.jpg", "preference" => -1},
-          %{"url" => "https://i.ytimg.com/vi/ABC123/img_2.jpg", "preference" => 1},
-          %{"url" => "https://i.ytimg.com/vi/ABC123/img_3.jpg", "preference" => -10},
-          %{"url" => "https://i.ytimg.com/vi/ABC123/img_4.webp", "preference" => 10}
-        ]
-      }
-
-      filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata)
-
-      assert filepath =~ ~r{/media_items/#{media_item.id}/img_2.jpg}
-    end
-
-    test "will fall back to a non-jpg if it has to", %{media_item: media_item} do
-      metadata = %{
-        "thumbnails" => [
-          %{"url" => "https://i.ytimg.com/vi/ABC123/img_1.webp", "preference" => -1}
-        ]
-      }
-
-      filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata)
-
-      assert filepath =~ ~r{/media_items/#{media_item.id}/img_1.webp}
-    end
-
-    test "does not require a preference field", %{media_item: media_item} do
-      metadata = %{
-        "thumbnails" => [
-          %{"url" => "https://i.ytimg.com/vi/ABC123/img_1.webp"}
-        ]
-      }
-
-      filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata)
-
-      assert filepath =~ ~r{/media_items/#{media_item.id}/img_1.webp}
-    end
-  end
-
-  describe "download_and_store_thumbnail_for/2 when not downloading thumbnails" do
-    test "returns nil if there are no thumbnails", %{media_item: media_item} do
-      metadata = %{"thumbnails" => []}
-
-      filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata)
-
-      assert filepath == nil
-    end
-
-    test "returns nil if there is no thumbnail field", %{media_item: media_item} do
-      metadata = %{}
-
-      filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata)
+      filepath = Helpers.download_and_store_thumbnail_for(media_item)
 
       assert filepath == nil
     end

test/pinchflat/yt_dlp/media_test.exs

@@ -7,8 +7,8 @@ defmodule Pinchflat.YtDlp.MediaTest do
 
   @media_url "https://www.youtube.com/watch?v=TiZPUDkDYbk"
 
-  describe "download/2" do
-    test "it calls the backend runner with the expected arguments" do
+  describe "download/3" do
+    test "calls the backend runner with the expected arguments" do
       expect(YtDlpRunnerMock, :run, fn @media_url, opts, ot, addl ->
         assert [:no_simulate] = opts
         assert "after_move:%()j" = ot
@@ -20,7 +20,7 @@ defmodule Pinchflat.YtDlp.MediaTest do
       assert {:ok, _} = Media.download(@media_url)
     end
 
-    test "it passes along additional options" do
+    test "passes along additional options" do
       expect(YtDlpRunnerMock, :run, fn _url, opts, _ot, addl ->
         assert [:no_simulate, :custom_arg] = opts
         assert [addl_arg: true] = addl
@@ -31,7 +31,7 @@ defmodule Pinchflat.YtDlp.MediaTest do
       assert {:ok, _} = Media.download(@media_url, [:custom_arg], addl_arg: true)
     end
 
-    test "it parses and returns the generated file as JSON" do
+    test "parses and returns the generated file as JSON" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
@@ -40,7 +40,7 @@ defmodule Pinchflat.YtDlp.MediaTest do
                Media.download(@media_url)
     end
 
-    test "it returns errors" do
+    test "returns errors" do
       expect(YtDlpRunnerMock, :run, fn _url, _opt, _ot, _addl ->
         {:error, "something"}
       end)
@@ -49,6 +49,37 @@ defmodule Pinchflat.YtDlp.MediaTest do
     end
   end
 
+  describe "download_thumbnail/2" do
+    test "calls the backend runner with the expected arguments" do
+      expect(YtDlpRunnerMock, :run, fn @media_url, opts, ot ->
+        assert opts == [:no_simulate, :skip_download, :write_thumbnail, {:convert_thumbnail, "jpg"}]
+        assert ot == "after_move:%()j"
+
+        {:ok, ""}
+      end)
+
+      assert {:ok, _} = Media.download_thumbnail(@media_url)
+    end
+
+    test "passes along additional options" do
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot ->
+        assert :custom_arg in opts
+
+        {:ok, "{}"}
+      end)
+
+      assert {:ok, _} = Media.download_thumbnail(@media_url, [:custom_arg])
+    end
+
+    test "returns errors" do
+      expect(YtDlpRunnerMock, :run, fn _url, _opt, _ot ->
+        {:error, "something"}
+      end)
+
+      assert {:error, "something"} = Media.download_thumbnail(@media_url)
+    end
+  end
+
   describe "get_media_attributes/1" do
     test "returns a list of video attributes" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->