Allow subtitle downloading (#11)

* Added subtitle options to media profile model

* Updated media profile form

* Adds subtitle-based options in options builder

* Updates metadata parser to include subtitles

* Adds subtitle_filepaths to media_item

* renamed video_filepath to media_filepath

* Added more fields to media profile show page

.iex.exs

@@ -0,0 +1,14 @@
+alias Pinchflat.Repo
+
+alias Pinchflat.Tasks.Task
+alias Pinchflat.Media.MediaItem
+alias Pinchflat.Media.MediaMetadata
+alias Pinchflat.MediaSource.Channel
+alias Pinchflat.Profiles.MediaProfile
+
+alias Pinchflat.Tasks
+alias Pinchflat.Media
+alias Pinchflat.Profiles
+alias Pinchflat.MediaSource
+
+alias Pinchflat.MediaClient.{ChannelDetails, VideoDownloader}

lib/pinchflat/media.ex

@@ -19,14 +19,14 @@ defmodule Pinchflat.Media do
 
   @doc """
   Returns a list of pending media_items for a given channel, where
-  pending means the `video_filepath` is `nil`.
+  pending means the `media_filepath` is `nil`.
 
   Returns [%MediaItem{}, ...].
   """
   def list_pending_media_items_for(%Channel{} = channel) do
     from(
       m in MediaItem,
-      where: m.channel_id == ^channel.id and is_nil(m.video_filepath)
+      where: m.channel_id == ^channel.id and is_nil(m.media_filepath)
     )
     |> Repo.all()
   end

lib/pinchflat/media/media_item.ex

@@ -11,12 +11,16 @@ defmodule Pinchflat.Media.MediaItem do
   alias Pinchflat.Media.MediaMetadata
 
   @required_fields ~w(media_id channel_id)a
-  @allowed_fields ~w(title media_id video_filepath channel_id)a
+  @allowed_fields ~w(title media_id media_filepath channel_id subtitle_filepaths)a
 
   schema "media_items" do
     field :title, :string
     field :media_id, :string
-    field :video_filepath, :string
+    field :media_filepath, :string
+    # This is an array of [iso-2 language, filepath] pairs. Probably could
+    # be an associated record, but I don't see the benefit right now.
+    # Will very likely revisit because I can't leave well-enough alone.
+    field :subtitle_filepaths, {:array, {:array, :string}}, default: []
 
     belongs_to :channel, Channel
 

lib/pinchflat/media_client/backends/yt_dlp/metadata_parser.ex

@@ -16,12 +16,34 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MetadataParser do
   Returns map()
   """
   def parse_for_media_item(metadata) do
-    %{
-      title: metadata["title"],
-      video_filepath: metadata["filepath"],
+    metadata_attrs = %{
       metadata: %{
         client_response: metadata
       }
     }
+
+    metadata_attrs
+    |> Map.merge(parse_media_metadata(metadata))
+    |> Map.merge(parse_subtitle_metadata(metadata))
+  end
+
+  defp parse_media_metadata(metadata) do
+    %{
+      title: metadata["title"],
+      media_filepath: metadata["filepath"]
+    }
+  end
+
+  defp parse_subtitle_metadata(metadata) do
+    subtitle_map = metadata["requested_subtitles"] || %{}
+    # IDEA: if needed, consider filtering out subtitles that don't exist on-disk
+    subtitle_filepaths =
+      subtitle_map
+      |> Enum.map(fn {lang, attrs} -> [lang, attrs["filepath"]] end)
+      |> Enum.sort(fn [lang_a, _], [lang_b, _] -> lang_a < lang_b end)
+
+    %{
+      subtitle_filepaths: subtitle_filepaths
+    }
   end
 end

lib/pinchflat/profiles/media_profile.ex

@@ -8,9 +8,24 @@ defmodule Pinchflat.Profiles.MediaProfile do
 
   alias Pinchflat.MediaSource.Channel
 
+  @allowed_fields ~w(
+    name
+    output_path_template
+    download_subs
+    download_auto_subs
+    embed_subs
+    sub_langs
+  )a
+
+  @required_fields ~w(name output_path_template)a
+
   schema "media_profiles" do
     field :name, :string
     field :output_path_template, :string
+    field :download_subs, :boolean, default: true
+    field :download_auto_subs, :boolean, default: true
+    field :embed_subs, :boolean, default: true
+    field :sub_langs, :string, default: "en"
 
     has_many :channels, Channel
 
@@ -20,8 +35,8 @@ defmodule Pinchflat.Profiles.MediaProfile do
   @doc false
   def changeset(media_profile, attrs) do
     media_profile
-    |> cast(attrs, [:name, :output_path_template])
-    |> validate_required([:name, :output_path_template])
+    |> cast(attrs, @allowed_fields)
+    |> validate_required(@required_fields)
     |> unique_constraint(:name)
   end
 end

lib/pinchflat/profiles/options/yt_dlp/option_builder.ex

@@ -15,22 +15,62 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilder do
         these options
   """
   def build(%MediaProfile{} = media_profile) do
-    {:ok, output_path} = OutputPathBuilder.build(media_profile.output_path_template)
-
     # NOTE: I'll be hardcoding most things for now (esp. options to help me test) -
     # add more configuration later as I build out the models. Walk before you can run!
 
     # NOTE: Looks like you can put different media types in different directories.
     # see: https://github.com/yt-dlp/yt-dlp#output-template
-    {:ok,
-     [
-       :embed_metadata,
-       :embed_thumbnail,
-       :embed_subs,
-       :no_progress,
-       sub_langs: "en.*",
-       output: Path.join(base_directory(), output_path)
-     ]}
+
+    built_options =
+      default_options() ++
+        subtitle_options(media_profile) ++
+        output_options(media_profile)
+
+    {:ok, built_options}
+  end
+
+  # This will be updated a lot as I add new options to profiles
+  defp default_options do
+    [
+      :embed_metadata,
+      :embed_thumbnail,
+      :no_progress
+    ]
+  end
+
+  defp subtitle_options(media_profile) do
+    mapped_struct = Map.from_struct(media_profile)
+
+    Enum.reduce(mapped_struct, [], fn attr, acc ->
+      case {attr, media_profile} do
+        {{:download_subs, true}, _} ->
+          # Force SRT for now - MAY provide as an option in the future
+          acc ++ [:write_subs, convert_subs: "srt"]
+
+        {{:download_auto_subs, true}, %{download_subs: true}} ->
+          acc ++ [:write_auto_subs]
+
+        {{:embed_subs, true}, _} ->
+          acc ++ [:embed_subs]
+
+        {{:sub_langs, sub_langs}, %{download_subs: true}} ->
+          acc ++ [sub_langs: sub_langs]
+
+        {{:sub_langs, sub_langs}, %{embed_subs: true}} ->
+          acc ++ [sub_langs: sub_langs]
+
+        _ ->
+          acc
+      end
+    end)
+  end
+
+  defp output_options(media_profile) do
+    {:ok, output_path} = OutputPathBuilder.build(media_profile.output_path_template)
+
+    [
+      output: Path.join(base_directory(), output_path)
+    ]
   end
 
   defp base_directory do

lib/pinchflat_web/controllers/media_profiles/media_profile_html/media_profile_form.html.heex

@@ -4,6 +4,10 @@
   </.error>
   <.input field={f[:name]} type="text" label="Name" />
   <.input field={f[:output_path_template]} type="text" label="Output path template" />
+  <.input field={f[:download_subs]} type="checkbox" label="Download Subs" />
+  <.input field={f[:download_auto_subs]} type="checkbox" label="Download Autogenerated Subs" />
+  <.input field={f[:embed_subs]} type="checkbox" label="Embed Subs" />
+  <.input field={f[:sub_langs]} type="text" label="Sub Langs" />
   <:actions>
     <.button>Save Media profile</.button>
   </:actions>

lib/pinchflat_web/controllers/media_profiles/media_profile_html/show.html.heex

@@ -9,8 +9,14 @@
 </.header>
 
 <.list>
-  <:item title="Name"><%= @media_profile.name %></:item>
-  <:item title="Output path template"><%= @media_profile.output_path_template %></:item>
+  <:item
+    :for={
+      attr <- ~w(name output_path_template download_subs download_auto_subs embed_subs sub_langs)a
+    }
+    title={attr}
+  >
+    <%= Map.get(@media_profile, attr) %>
+  </:item>
 </.list>
 
 <.back navigate={~p"/media_profiles"}>Back to media_profiles</.back>

priv/repo/migrations/20240131042824_add_subtitle_options_to_media_profiles.exs

@@ -0,0 +1,12 @@
+defmodule Pinchflat.Repo.Migrations.AddSubtitleOptionsToMediaProfiles do
+  use Ecto.Migration
+
+  def change do
+    alter table(:media_profiles) do
+      add :download_subs, :boolean, default: true, null: false
+      add :download_auto_subs, :boolean, default: true, null: false
+      add :embed_subs, :boolean, default: true, null: false
+      add :sub_langs, :string, default: "en", null: false
+    end
+  end
+end

priv/repo/migrations/20240131182114_add_subtitles_to_media_item.exs

@@ -0,0 +1,9 @@
+defmodule Pinchflat.Repo.Migrations.AddSubtitleFilepathsToMediaItem do
+  use Ecto.Migration
+
+  def change do
+    alter table(:media_items) do
+      add :subtitle_filepaths, {:array, {:array, :string}}, default: []
+    end
+  end
+end

priv/repo/migrations/20240131183153_rename_video_filepath_on_media_items.exs

@@ -0,0 +1,7 @@
+defmodule Pinchflat.Repo.Migrations.RenameVideoFilepathOnMediaItems do
+  use Ecto.Migration
+
+  def change do
+    rename table(:media_items), :video_filepath, to: :media_filepath
+  end
+end

test/pinchflat/media_client/backends/yt_dlp/metadata_parser_test.exs

@@ -22,12 +22,12 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MediaParserTest do
      }}
   end
 
-  describe "parse_for_media_item/1" do
+  describe "parse_for_media_item/1 when testing media metadata" do
     test "it extracts the video filepath", %{metadata: metadata} do
       result = Parser.parse_for_media_item(metadata)
 
-      assert String.contains?(result.video_filepath, "bwRHIkYqYJo")
-      assert String.ends_with?(result.video_filepath, ".mkv")
+      assert String.contains?(result.media_filepath, "bwRHIkYqYJo")
+      assert String.ends_with?(result.media_filepath, ".mkv")
     end
 
     test "it extracts the title", %{metadata: metadata} do
@@ -42,4 +42,45 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MediaParserTest do
       assert result.metadata.client_response == metadata
     end
   end
+
+  describe "parse_for_media_item/1 when testing subtitle metadata" do
+    test "extracts the subtitle filepaths", %{metadata: metadata} do
+      result = Parser.parse_for_media_item(metadata)
+
+      assert [["de", german_filepath], ["en", english_filepath]] = result.subtitle_filepaths
+
+      assert String.ends_with?(english_filepath, ".en.srt")
+      assert String.ends_with?(german_filepath, ".de.srt")
+    end
+
+    test "sorts the subtitle filepaths by language", %{metadata: metadata} do
+      metadata =
+        Map.put(metadata, "requested_subtitles", %{
+          "en" => %{"filepath" => "en.srt"},
+          "za" => %{"filepath" => "za.srt"},
+          "de" => %{"filepath" => "de.srt"},
+          "al" => %{"filepath" => "al.srt"}
+        })
+
+      result = Parser.parse_for_media_item(metadata)
+
+      assert [["al", _], ["de", _], ["en", _], ["za", _]] = result.subtitle_filepaths
+    end
+
+    test "doesn't freak out if the video has no subtitles", %{metadata: metadata} do
+      metadata = Map.put(metadata, "requested_subtitles", %{})
+
+      result = Parser.parse_for_media_item(metadata)
+
+      assert result.subtitle_filepaths == []
+    end
+
+    test "doesn't freak out if the requested_subtitles key is missing", %{metadata: metadata} do
+      metadata = Map.delete(metadata, "requested_subtitles")
+
+      result = Parser.parse_for_media_item(metadata)
+
+      assert result.subtitle_filepaths == []
+    end
+  end
 end

test/pinchflat/media_client/video_downloader_test.exs

@@ -10,7 +10,7 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do
   setup do
     media_item =
       Repo.preload(
-        media_item_fixture(%{title: nil, video_filepath: nil}),
+        media_item_fixture(%{title: nil, media_filepath: nil}),
         [:metadata, channel: :media_profile]
       )
 
@@ -33,10 +33,11 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      assert %{video_filepath: nil, title: nil} = media_item
+      assert %{media_filepath: nil, title: nil, subtitle_filepaths: []} = media_item
       assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
-      assert updated_media_item.video_filepath
+      assert updated_media_item.media_filepath
       assert updated_media_item.title
+      assert length(updated_media_item.subtitle_filepaths) > 0
     end
 
     test "it saves the metadata to the database", %{media_item: media_item} do

test/pinchflat/media_test.exs

@@ -8,7 +8,7 @@ defmodule Pinchflat.MediaTest do
   alias Pinchflat.Media
   alias Pinchflat.Media.MediaItem
 
-  @invalid_attrs %{title: nil, media_id: nil, video_filepath: nil}
+  @invalid_attrs %{title: nil, media_id: nil, media_filepath: nil}
 
   describe "schema" do
     test "media_metadata is deleted when media_item is deleted" do
@@ -32,18 +32,18 @@ defmodule Pinchflat.MediaTest do
   describe "list_pending_media_items_for/1" do
     test "it returns pending media_items for a given channel" do
       channel = channel_fixture()
-      media_item = media_item_fixture(%{channel_id: channel.id, video_filepath: nil})
+      media_item = media_item_fixture(%{channel_id: channel.id, media_filepath: nil})
 
       assert Media.list_pending_media_items_for(channel) == [media_item]
     end
 
-    test "it does not return media_items with video_filepath" do
+    test "it does not return media_items with media_filepath" do
       channel = channel_fixture()
 
       _media_item =
         media_item_fixture(%{
           channel_id: channel.id,
-          video_filepath: "/video/#{Faker.File.file_name(:video)}"
+          media_filepath: "/video/#{Faker.File.file_name(:video)}"
         })
 
       assert Media.list_pending_media_items_for(channel) == []
@@ -62,14 +62,14 @@ defmodule Pinchflat.MediaTest do
       valid_attrs = %{
         media_id: Faker.String.base64(12),
         title: Faker.Commerce.product_name(),
-        video_filepath: "/video/#{Faker.File.file_name(:video)}",
+        media_filepath: "/video/#{Faker.File.file_name(:video)}",
         channel_id: channel_fixture().id
       }
 
       assert {:ok, %MediaItem{} = media_item} = Media.create_media_item(valid_attrs)
       assert media_item.title == valid_attrs.title
       assert media_item.media_id == valid_attrs.media_id
-      assert media_item.video_filepath == valid_attrs.video_filepath
+      assert media_item.media_filepath == valid_attrs.media_filepath
     end
 
     test "creating with invalid data returns error changeset" do
@@ -84,14 +84,14 @@ defmodule Pinchflat.MediaTest do
       update_attrs = %{
         media_id: Faker.String.base64(12),
         title: Faker.Commerce.product_name(),
-        video_filepath: "/video/#{Faker.File.file_name(:video)}",
+        media_filepath: "/video/#{Faker.File.file_name(:video)}",
         channel_id: channel_fixture().id
       }
 
       assert {:ok, %MediaItem{} = media_item} = Media.update_media_item(media_item, update_attrs)
       assert media_item.title == update_attrs.title
       assert media_item.media_id == update_attrs.media_id
-      assert media_item.video_filepath == update_attrs.video_filepath
+      assert media_item.media_filepath == update_attrs.media_filepath
     end
 
     test "updating with invalid data returns error changeset" do

test/pinchflat/profiles/options/yt_dlp/option_builder_test.exs

@@ -15,4 +15,83 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do
       assert {:output, "/tmp/videos/%(title)S.%(ext)s"} in res
     end
   end
+
+  describe "build/1 when testing subtitle options" do
+    test "includes :write_subs option when specified" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      assert :write_subs in res
+    end
+
+    test "forces SRT format when download_subs is true" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      assert {:convert_subs, "srt"} in res
+    end
+
+    test "includes :write_auto_subs option when specified" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true, download_auto_subs: true}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      assert :write_auto_subs in res
+    end
+
+    test "doesn't include :write_auto_subs option when download_subs is false" do
+      media_profile = %MediaProfile{@media_profile | download_subs: false, download_auto_subs: true}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      refute :write_auto_subs in res
+    end
+
+    test "includes :embed_subs option when specified" do
+      media_profile = %MediaProfile{@media_profile | embed_subs: true}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      assert :embed_subs in res
+    end
+
+    test "includes sub_langs option when download_subs is true" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true, sub_langs: "en"}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      assert {:sub_langs, "en"} in res
+    end
+
+    test "includes sub_langs option when embed_subs is true" do
+      media_profile = %MediaProfile{@media_profile | embed_subs: true, sub_langs: "en"}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      assert {:sub_langs, "en"} in res
+    end
+
+    test "doesn't include sub_langs option when neither downloading nor embedding" do
+      media_profile = %MediaProfile{
+        @media_profile
+        | embed_subs: false,
+          download_subs: false,
+          sub_langs: "en"
+      }
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      refute {:sub_langs, "en"} in res
+    end
+
+    test "other struct attributes are ignored" do
+      media_profile = %MediaProfile{@media_profile | id: -1}
+
+      assert {:ok, res} = OptionBuilder.build(media_profile)
+
+      refute {:id, -1} in res
+    end
+  end
 end

test/pinchflat/workers/media_indexing_worker_test.exs

@@ -50,7 +50,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1"} end)
 
       channel = channel_fixture(index_frequency_minutes: 10)
-      media_item_fixture(%{channel_id: channel.id, video_filepath: nil})
+      media_item_fixture(%{channel_id: channel.id, media_filepath: nil})
       perform_job(MediaIndexingWorker, %{id: channel.id})
 
       assert [_, _] = all_enqueued(worker: VideoDownloadWorker)

test/pinchflat/workers/video_download_worker_test.exs

@@ -11,7 +11,7 @@ defmodule Pinchflat.Workers.VideoDownloadWorkerTest do
   setup do
     media_item =
       Repo.preload(
-        media_item_fixture(%{video_filepath: nil}),
+        media_item_fixture(%{media_filepath: nil}),
         [:metadata, channel: :media_profile]
       )
 
@@ -24,9 +24,9 @@ defmodule Pinchflat.Workers.VideoDownloadWorkerTest do
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      assert media_item.video_filepath == nil
+      assert media_item.media_filepath == nil
       perform_job(VideoDownloadWorker, %{id: media_item.id})
-      assert Repo.reload(media_item).video_filepath != nil
+      assert Repo.reload(media_item).media_filepath != nil
     end
 
     test "it saves the metadata to the media_item", %{media_item: media_item} do

test/support/fixtures/media_fixtures.ex

@@ -15,7 +15,7 @@ defmodule Pinchflat.MediaFixtures do
       |> Enum.into(%{
         media_id: Faker.String.base64(12),
         title: Faker.Commerce.product_name(),
-        video_filepath: "/video/#{Faker.File.file_name(:video)}",
+        media_filepath: "/video/#{Faker.File.file_name(:video)}",
         channel_id: MediaSourceFixtures.channel_fixture().id
       })
       |> Pinchflat.Media.create_media_item()