From 8b2d7ec759c7a7a6ce8fc0becf86a4dfc0d6c894 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Sat, 2 Mar 2024 11:32:42 -0800 Subject: [PATCH] Updated download options to take a media item; allowed for specifying custom output path template options --- .../media_client/video_downloader.ex | 8 +- .../options/yt_dlp/download_option_builder.ex | 30 ++-- .../options/yt_dlp/output_path_builder.ex | 9 +- .../yt_dlp/output_path_builder_test.exs | 8 +- .../yt_dlp/download_option_builder_test.exs | 158 +++++++++--------- 5 files changed, 117 insertions(+), 96 deletions(-) diff --git a/lib/pinchflat/media_client/video_downloader.ex b/lib/pinchflat/media_client/video_downloader.ex index 4ab205f..d1028e9 100644 --- a/lib/pinchflat/media_client/video_downloader.ex +++ b/lib/pinchflat/media_client/video_downloader.ex @@ -11,7 +11,6 @@ defmodule Pinchflat.MediaClient.VideoDownloader do alias Pinchflat.Repo alias Pinchflat.Media alias Pinchflat.Media.MediaItem - alias Pinchflat.Profiles.MediaProfile alias Pinchflat.MediaClient.Backends.YtDlp.Video, as: YtDlpVideo alias Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder, as: YtDlpDownloadOptionBuilder @@ -31,9 +30,8 @@ defmodule Pinchflat.MediaClient.VideoDownloader do """ def download_for_media_item(%MediaItem{} = media_item, backend \\ :yt_dlp) do item_with_preloads = Repo.preload(media_item, [:metadata, source: :media_profile]) - media_profile = item_with_preloads.source.media_profile - case download_for_media_profile(media_item.original_url, media_profile, backend) do + case download_with_options(media_item.original_url, item_with_preloads, backend) do {:ok, parsed_json} -> {parser, helpers} = metadata_parsers(backend) @@ -57,10 +55,10 @@ defmodule Pinchflat.MediaClient.VideoDownloader do end end - defp download_for_media_profile(url, %MediaProfile{} = media_profile, backend) do + defp download_with_options(url, item_with_preloads, backend) do option_builder = option_builder(backend) video_backend = video_backend(backend) - {:ok, options} = option_builder.build(media_profile) + {:ok, options} = option_builder.build(item_with_preloads) video_backend.download(url, options) end diff --git a/lib/pinchflat/profiles/options/yt_dlp/download_option_builder.ex b/lib/pinchflat/profiles/options/yt_dlp/download_option_builder.ex index 02d735b..fd12193 100644 --- a/lib/pinchflat/profiles/options/yt_dlp/download_option_builder.ex +++ b/lib/pinchflat/profiles/options/yt_dlp/download_option_builder.ex @@ -5,21 +5,17 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder do IDEA: consider making this a behaviour so I can add other backends later """ - alias Pinchflat.Profiles.MediaProfile + alias Pinchflat.Media.MediaItem alias Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder @doc """ - Builds the options for yt-dlp to download media based on the given media profile. + Builds the options for yt-dlp to download media based on the given media's profile. IDEA: consider adding the ability to pass in a second argument to override these options """ - def build(%MediaProfile{} = media_profile) do - # 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 + def build(%MediaItem{} = media_item_with_preloads) do + media_profile = media_item_with_preloads.source.media_profile built_options = default_options() ++ @@ -27,12 +23,11 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder do thumbnail_options(media_profile) ++ metadata_options(media_profile) ++ quality_options(media_profile) ++ - output_options(media_profile) + output_options(media_item_with_preloads) {:ok, built_options} end - # This will be updated a lot as I add new options to profiles defp default_options do [:no_progress, :windows_filenames] end @@ -101,14 +96,25 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder do end end - defp output_options(media_profile) do - {:ok, output_path} = OutputPathBuilder.build(media_profile.output_path_template) + 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.join(base_directory(), output_path) ] end + defp output_options_map(media_item_with_preloads) do + source = media_item_with_preloads.source + + %{ + "source_friendly_name" => source.friendly_name, + "source_collection_type" => source.collection_type + } + end + defp base_directory do Application.get_env(:pinchflat, :media_directory) end diff --git a/lib/pinchflat/profiles/options/yt_dlp/output_path_builder.ex b/lib/pinchflat/profiles/options/yt_dlp/output_path_builder.ex index dadb9fe..373aca8 100644 --- a/lib/pinchflat/profiles/options/yt_dlp/output_path_builder.ex +++ b/lib/pinchflat/profiles/options/yt_dlp/output_path_builder.ex @@ -8,13 +8,16 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder do alias Pinchflat.RenderedString.Parser, as: TemplateParser @doc """ - Builds the actual final filepath from a given template. + Builds the actual final filepath from a given template. Optionally, you can pass in + a map of additional options to be used in the template. Translates liquid-style templates into yt-dlp-style templates, leaving yt-dlp syntax intact. """ - def build(template_string) do - TemplateParser.parse(template_string, custom_yt_dlp_option_map(), &identifier_fn/2) + def build(template_string, additional_template_options \\ %{}) do + combined_options = Map.merge(custom_yt_dlp_option_map(), additional_template_options) + + TemplateParser.parse(template_string, combined_options, &identifier_fn/2) end # The `nil` case simply wraps the identifier in yt-dlp-style syntax. This assumes that diff --git a/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs b/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs index 8915639..f924981 100644 --- a/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs +++ b/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs @@ -3,7 +3,7 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.OutputPathBuilderTest do alias Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder - describe "build/1" do + describe "build/2" do test "it expands 'standard' curly brace variables in the template" do assert {:ok, res} = OutputPathBuilder.build("/videos/{{ title }}.{{ ext }}") @@ -16,6 +16,12 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.OutputPathBuilderTest do assert res == "/videos/%(upload_date>%Y)S.%(ext)S" end + test "it respects additional options" do + assert {:ok, res} = OutputPathBuilder.build("/videos/{{ custom }}.{{ ext }}", %{"custom" => "test"}) + + assert res == "/videos/test.%(ext)S" + end + test "it leaves yt-dlp variables alone" do assert {:ok, res} = OutputPathBuilder.build("/videos/%(title)s.%(ext)s") diff --git a/test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs b/test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs index 69e7f12..357b072 100644 --- a/test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs +++ b/test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs @@ -1,24 +1,40 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do - use ExUnit.Case, async: true + use Pinchflat.DataCase + import Pinchflat.MediaFixtures + import Pinchflat.ProfilesFixtures + import Pinchflat.SourcesFixtures - alias Pinchflat.Profiles.MediaProfile + alias Pinchflat.Profiles alias Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder - @media_profile %MediaProfile{ - output_path_template: "{{ title }}.%(ext)s" - } + setup do + media_profile = media_profile_fixture(%{output_path_template: "{{ title }}.%(ext)s"}) + source = source_fixture(%{media_profile_id: media_profile.id, friendly_name: "my source"}) + media_item = Repo.preload(media_item_fixture(source_id: source.id), source: :media_profile) - describe "build/1" do - test "it generates an expanded output path based on the given template" do - assert {:ok, res} = DownloadOptionBuilder.build(@media_profile) + {:ok, media_item: media_item} + end + + describe "build/1 when testing output options" do + test "it generates an expanded output path based on the given template", %{media_item: media_item} do + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert {:output, "/tmp/test/videos/%(title)S.%(ext)s"} in res end + + test "it respects custom output path options", %{media_item: media_item} do + media_item = + update_media_profile_attribute(media_item, %{output_path_template: "{{ source_friendly_name }}.%(ext)s"}) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + assert {:output, "/tmp/test/videos/#{media_item.source.friendly_name}.%(ext)s"} in res + end end describe "build/1 when testing default options" do - test "it includes default options" do - assert {:ok, res} = DownloadOptionBuilder.build(@media_profile) + test "it includes default options", %{media_item: media_item} do + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert :no_progress in res assert :windows_filenames in res @@ -26,109 +42,93 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do 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} + test "includes :write_subs option when specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_subs: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert :write_subs in res end - test "forces SRT format when download_subs is true" do - media_profile = %MediaProfile{@media_profile | download_subs: true} + test "forces SRT format when download_subs is true", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_subs: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) 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} + test "includes :write_auto_subs option when specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_subs: true, download_auto_subs: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) 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} + test "doesn't include :write_auto_subs option when download_subs is false", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_subs: false, download_auto_subs: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) refute :write_auto_subs in res end - test "includes :embed_subs option when specified" do - media_profile = %MediaProfile{@media_profile | embed_subs: true} + test "includes :embed_subs option when specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{embed_subs: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) 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"} + test "includes sub_langs option when download_subs is true", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_subs: true, sub_langs: "en"}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) 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"} + test "includes sub_langs option when embed_subs is true", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{embed_subs: true, sub_langs: "en"}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) 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" - } + test "doesn't include sub_langs option when neither downloading nor embedding", %{media_item: media_item} do + media_item = + update_media_profile_attribute(media_item, %{embed_subs: false, download_subs: false, sub_langs: "en"}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) refute {:sub_langs, "en"} in res end - - test "other struct attributes are ignored" do - media_profile = %MediaProfile{@media_profile | id: -1} - - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) - - refute {:id, -1} in res - end end describe "build/1 when testing thumbnail options" do - test "includes :write_thumbnail option when specified" do - media_profile = %MediaProfile{@media_profile | download_thumbnail: true} + test "includes :write_thumbnail option when specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert :write_thumbnail in res end - test "includes :embed_thumbnail option when specified" do - media_profile = %MediaProfile{@media_profile | embed_thumbnail: true} + test "includes :embed_thumbnail option when specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{embed_thumbnail: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert :embed_thumbnail in res end - test "doesn't include these options when not specified" do - media_profile = %MediaProfile{ - @media_profile - | embed_thumbnail: false, - download_thumbnail: false - } + test "doesn't include these options when not specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{embed_thumbnail: false, download_thumbnail: false}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) refute :write_thumbnail in res refute :embed_thumbnail in res @@ -136,31 +136,27 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do end describe "build/1 when testing metadata options" do - test "includes :write_info_json option when specified" do - media_profile = %MediaProfile{@media_profile | download_metadata: true} + test "includes :write_info_json option when specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_metadata: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert :write_info_json in res assert :clean_info_json in res end - test "includes :embed_metadata option when specified" do - media_profile = %MediaProfile{@media_profile | embed_metadata: true} + test "includes :embed_metadata option when specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{embed_metadata: true}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert :embed_metadata in res end - test "doesn't include these options when not specified" do - media_profile = %MediaProfile{ - @media_profile - | embed_metadata: false, - download_metadata: false - } + test "doesn't include these options when not specified", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{embed_metadata: false, download_metadata: false}) - assert {:ok, res} = DownloadOptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_item) refute :write_info_json in res refute :clean_info_json in res @@ -169,10 +165,22 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do end describe "build/1 when testing quality options" do - test "it includes quality options" do - assert {:ok, res} = DownloadOptionBuilder.build(@media_profile) + test "it includes quality options", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{preferred_resolution: :"1080p"}) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert {:format_sort, "res:1080,+codec:avc:m4a"} in res end end + + defp update_media_profile_attribute(media_item_with_preloads, attrs) do + media_item_with_preloads.source.media_profile + |> Profiles.change_media_profile(attrs) + |> Repo.update!() + + media_item_with_preloads + |> Repo.reload() + |> Repo.preload(source: :media_profile) + end end