Append -thumb to thumbnails when downloading (#87)

* Appended -thumb to thumbnails when downloading

* Added code to compensate for yt-dlp bug

lib/pinchflat/downloading/download_option_builder.ex

@@ -18,7 +18,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
     built_options =
       default_options() ++
         subtitle_options(media_profile) ++
-        thumbnail_options(media_profile) ++
+        thumbnail_options(media_item_with_preloads) ++
         metadata_options(media_profile) ++
         quality_options(media_profile) ++
         output_options(media_item_with_preloads)
@@ -57,13 +57,16 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
     end)
   end
 
-  defp thumbnail_options(media_profile) do
+  defp thumbnail_options(media_item_with_preloads) do
+    media_profile = media_item_with_preloads.source.media_profile
     mapped_struct = Map.from_struct(media_profile)
 
     Enum.reduce(mapped_struct, [], fn attr, acc ->
       case attr do
         {:download_thumbnail, true} ->
-          acc ++ [:write_thumbnail, convert_thumbnail: "jpg"]
+          thumbnail_save_location = determine_thumbnail_location(media_item_with_preloads)
+
+          acc ++ [:write_thumbnail, convert_thumbnail: "jpg", output: "thumbnail:#{thumbnail_save_location}"]
 
         {:embed_thumbnail, true} ->
           acc ++ [:embed_thumbnail, convert_thumbnail: "jpg"]
@@ -102,15 +105,20 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
   end
 
   defp output_options(media_item_with_preloads) do
-    media_profile = media_item_with_preloads.source.media_profile
-    additional_options_map = output_options_map(media_item_with_preloads)
-    {:ok, output_path} = OutputPathBuilder.build(media_profile.output_path_template, additional_options_map)
+    output_path_template = media_item_with_preloads.source.media_profile.output_path_template
 
     [
-      output: Path.join(base_directory(), output_path)
+      output: build_output_path(output_path_template, media_item_with_preloads)
     ]
   end
 
+  defp build_output_path(string, media_item_with_preloads) do
+    additional_options_map = output_options_map(media_item_with_preloads)
+    {:ok, output_path} = OutputPathBuilder.build(string, additional_options_map)
+
+    Path.join(base_directory(), output_path)
+  end
+
   defp output_options_map(media_item_with_preloads) do
     source = media_item_with_preloads.source
 
@@ -120,6 +128,19 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
     }
   end
 
+  # I don't love the string manipulation here, but what can ya' do.
+  # It's dependent on the output_path_template being a string ending `.{{ ext }}`
+  # (or equivalent), but that's validated by the MediaProfile schema.
+  defp determine_thumbnail_location(media_item_with_preloads) do
+    output_path_template = media_item_with_preloads.source.media_profile.output_path_template
+
+    output_path_template
+    |> String.split(~r{\.}, include_captures: true)
+    |> List.insert_at(-3, "-thumb")
+    |> Enum.join()
+    |> build_output_path(media_item_with_preloads)
+  end
+
   defp base_directory do
     Application.get_env(:pinchflat, :media_directory)
   end

lib/pinchflat/metadata/metadata_parser.ex

@@ -54,9 +54,22 @@ defmodule Pinchflat.Metadata.MetadataParser do
       |> Enum.reverse()
       |> Enum.find_value(fn attrs -> attrs["filepath"] end)
 
-    %{
-      thumbnail_filepath: thumbnail_filepath
-    }
+    if thumbnail_filepath do
+      # NOTE: whole ordeal needed due to a bug I found in yt-dlp
+      # https://github.com/yt-dlp/yt-dlp/issues/9445
+      # Can be reverted to remove this entire conditional once fixed
+      %{
+        thumbnail_filepath:
+          thumbnail_filepath
+          |> String.split(~r{\.}, include_captures: true)
+          |> List.insert_at(-3, "-thumb")
+          |> Enum.join()
+      }
+    else
+      %{
+        thumbnail_filepath: thumbnail_filepath
+      }
+    end
   end
 
   defp parse_infojson_metadata(metadata) do

lib/pinchflat/profiles/media_profile.ex

@@ -67,6 +67,12 @@ defmodule Pinchflat.Profiles.MediaProfile do
     media_profile
     |> cast(attrs, @allowed_fields)
     |> validate_required(@required_fields)
+    # Ensures it ends with `.{{ ext }}` or `.%(ext)s` or similar (with a little wiggle room)
+    |> validate_format(:output_path_template, ext_regex(), message: "must end with .{{ ext }}")
     |> unique_constraint(:name)
   end
+
+  defp ext_regex do
+    ~r/\.({{ ?ext ?}}|%\( ?ext ?\)[sS])$/
+  end
 end

test/pinchflat/downloading/download_option_builder_test.exs

@@ -125,7 +125,15 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
       assert :write_thumbnail in res
     end
 
-    test "convertes thumbnail to jpg when download_thumbnail is true", %{media_item: media_item} do
+    test "appends -thumb to the thumbnail name when download_thumbnail is true", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true})
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
+
+      assert {:output, "thumbnail:/tmp/test/media/%(title)S-thumb.%(ext)s"} in res
+    end
+
+    test "converts thumbnail to jpg when download_thumbnail is true", %{media_item: media_item} do
       media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true})
 
       assert {:ok, res} = DownloadOptionBuilder.build(media_item)

test/pinchflat/metadata/metadata_parser_test.exs

@@ -94,6 +94,15 @@ defmodule Pinchflat.Metadata.MetadataParserTest do
       assert String.ends_with?(result.thumbnail_filepath, ".webp")
     end
 
+    # NOTE: this can be removed once this bug is fixed
+    # https://github.com/yt-dlp/yt-dlp/issues/9445
+    # and the associated conditional in the parser is removed
+    test "automatically appends `-thumb` to the thumbnail filename", %{metadata: metadata} do
+      result = Parser.parse_for_media_item(metadata)
+
+      assert String.contains?(result.thumbnail_filepath, "-thumb.webp")
+    end
+
     test "doesn't freak out if the media has no thumbnails", %{metadata: metadata} do
       metadata = Map.put(metadata, "thumbnails", %{})
 

test/pinchflat/profiles_test.exs

@@ -26,11 +26,11 @@ defmodule Pinchflat.ProfilesTest do
 
   describe "create_media_profile/1" do
     test "creation with valid data creates a media_profile" do
-      valid_attrs = %{name: "some name", output_path_template: "some output_path_template"}
+      valid_attrs = %{name: "some name", output_path_template: "output_template.{{ ext }}"}
 
       assert {:ok, %MediaProfile{} = media_profile} = Profiles.create_media_profile(valid_attrs)
       assert media_profile.name == "some name"
-      assert media_profile.output_path_template == "some output_path_template"
+      assert media_profile.output_path_template == "output_template.{{ ext }}"
     end
 
     test "creation with invalid data returns error changeset" do
@@ -44,14 +44,14 @@ defmodule Pinchflat.ProfilesTest do
 
       update_attrs = %{
         name: "some updated name",
-        output_path_template: "some updated output_path_template"
+        output_path_template: "new_output_template.{{ ext }}"
       }
 
       assert {:ok, %MediaProfile{} = media_profile} =
                Profiles.update_media_profile(media_profile, update_attrs)
 
       assert media_profile.name == "some updated name"
-      assert media_profile.output_path_template == "some updated output_path_template"
+      assert media_profile.output_path_template == "new_output_template.{{ ext }}"
     end
 
     test "updating with invalid data returns error changeset" do
@@ -132,5 +132,41 @@ defmodule Pinchflat.ProfilesTest do
       media_profile = media_profile_fixture()
       assert %Ecto.Changeset{} = Profiles.change_media_profile(media_profile)
     end
+
+    test "it ensures the media profile's output template ends with an extension" do
+      valid_templates = [
+        "output_template.{{ ext }}",
+        "output_template.{{ext}}",
+        "output_template.%(ext)s",
+        "output_template.%(ext)S",
+        "output_template.%( ext )s",
+        "output_template.%( ext )S"
+      ]
+
+      for template <- valid_templates do
+        cs = Profiles.change_media_profile(%MediaProfile{}, %{name: "a", output_path_template: template})
+
+        assert cs.valid?
+      end
+    end
+
+    test "it does not allow invalid output templates" do
+      invalid_templates = [
+        "output_template.{{ ext }}.something",
+        "output_template.{{   ext   }}",
+        "output_template{{ ext }}",
+        "output_template.%(ext)s.something",
+        "output_template.txt",
+        "output_template%(ext)s",
+        "output_template.%(nope)s",
+        "output_template"
+      ]
+
+      for template <- invalid_templates do
+        cs = Profiles.change_media_profile(%MediaProfile{}, %{name: "a", output_path_template: template})
+
+        refute cs.valid?
+      end
+    end
   end
 end

test/pinchflat_web/controllers/media_profile_controller_test.exs

@@ -8,10 +8,10 @@ defmodule PinchflatWeb.MediaProfileControllerTest do
   alias Pinchflat.Repo
   alias Pinchflat.Settings
 
-  @create_attrs %{name: "some name", output_path_template: "some output_path_template"}
+  @create_attrs %{name: "some name", output_path_template: "output_template.{{ ext }}"}
   @update_attrs %{
     name: "some updated name",
-    output_path_template: "some updated output_path_template"
+    output_path_template: "new_output_template.{{ ext }}"
   }
   @invalid_attrs %{name: nil, output_path_template: nil}