From d60f2d1422245758051b98a74b9d82c99943a290 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Tue, 6 Feb 2024 11:39:11 -0800 Subject: [PATCH] Added release-type options to media profile; built option parser for indexing operations --- .gitignore | 1 + lib/pinchflat/media_client/source_details.ex | 28 ++++- .../media_client/video_downloader.ex | 4 +- lib/pinchflat/profiles/media_profile.ex | 10 ++ ..._builder.ex => download_option_builder.ex} | 6 +- .../options/yt_dlp/index_option_builder.ex | 52 +++++++++ .../options/yt_dlp/output_path_builder.ex | 5 + ...lease_type_behaviour_to_media_profiles.exs | 10 ++ .../media_client/source_details_test.exs | 35 +++++- ...t.exs => download_option_builder_test.exs} | 30 ++--- .../yt_dlp/index_option_builder_test.exs | 104 ++++++++++++++++++ 11 files changed, 260 insertions(+), 25 deletions(-) rename lib/pinchflat/profiles/options/yt_dlp/{option_builder.ex => download_option_builder.ex} (91%) create mode 100644 lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex create mode 100644 priv/repo/migrations/20240205215634_add_release_type_behaviour_to_media_profiles.exs rename test/pinchflat/profiles/options/yt_dlp/{option_builder_test.exs => download_option_builder_test.exs} (74%) create mode 100644 test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs diff --git a/.gitignore b/.gitignore index 4ef91f6..7ab4c72 100644 --- a/.gitignore +++ b/.gitignore @@ -39,3 +39,4 @@ npm-debug.log /.elixir_ls .env .DS_Store +scratchpad.md diff --git a/lib/pinchflat/media_client/source_details.ex b/lib/pinchflat/media_client/source_details.ex index 0294600..dcef2d5 100644 --- a/lib/pinchflat/media_client/source_details.ex +++ b/lib/pinchflat/media_client/source_details.ex @@ -6,10 +6,14 @@ defmodule Pinchflat.MediaClient.SourceDetails do it open-ish for future expansion (just in case). """ + alias Pinchflat.Repo + alias Pinchflat.MediaSource.Source + alias Pinchflat.MediaClient.Backends.YtDlp.VideoCollection, as: YtDlpSource + alias Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder, as: YtDlpIndexOptionBuilder @doc """ - Gets a source's ID and name from its URL, using the given backend. + Gets a source's ID and name from its URL using the given backend. Returns {:ok, map()} | {:error, any, ...}. """ @@ -18,11 +22,23 @@ defmodule Pinchflat.MediaClient.SourceDetails do end @doc """ - Returns a list of video IDs for the given source URL, using the given backend. + Returns a list of video IDs for the given source URL OR source record using the given backend. + + If passing a source record, the call to the backend may have custom options applied based on + the `option_builder`. Returns {:ok, list(binary())} | {:error, any, ...}. """ - def get_video_ids(source_url, backend \\ :yt_dlp) do + def get_video_ids(sourceable, backend \\ :yt_dlp) + + def get_video_ids(%Source{} = source, backend) do + media_profile = Repo.preload(source, :media_profile).media_profile + {:ok, options} = option_builder(backend).build(media_profile) + + source_module(backend).get_video_ids(source.collection_id, options) + end + + def get_video_ids(source_url, backend) when is_binary(source_url) do source_module(backend).get_video_ids(source_url) end @@ -31,4 +47,10 @@ defmodule Pinchflat.MediaClient.SourceDetails do :yt_dlp -> YtDlpSource end end + + defp option_builder(backend) do + case backend do + :yt_dlp -> YtDlpIndexOptionBuilder + end + end end diff --git a/lib/pinchflat/media_client/video_downloader.ex b/lib/pinchflat/media_client/video_downloader.ex index 68d7621..7bb670e 100644 --- a/lib/pinchflat/media_client/video_downloader.ex +++ b/lib/pinchflat/media_client/video_downloader.ex @@ -14,7 +14,7 @@ defmodule Pinchflat.MediaClient.VideoDownloader do alias Pinchflat.Profiles.MediaProfile alias Pinchflat.MediaClient.Backends.YtDlp.Video, as: YtDlpVideo - alias Pinchflat.Profiles.Options.YtDlp.OptionBuilder, as: YtDlpOptionBuilder + alias Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder, as: YtDlpDownloadOptionBuilder alias Pinchflat.MediaClient.Backends.YtDlp.MetadataParser, as: YtDlpMetadataParser @doc """ @@ -52,7 +52,7 @@ defmodule Pinchflat.MediaClient.VideoDownloader do defp option_builder(backend) do case backend do - :yt_dlp -> YtDlpOptionBuilder + :yt_dlp -> YtDlpDownloadOptionBuilder end end diff --git a/lib/pinchflat/profiles/media_profile.ex b/lib/pinchflat/profiles/media_profile.ex index 579aa9c..1a81738 100644 --- a/lib/pinchflat/profiles/media_profile.ex +++ b/lib/pinchflat/profiles/media_profile.ex @@ -17,6 +17,8 @@ defmodule Pinchflat.Profiles.MediaProfile do sub_langs download_thumbnail embed_thumbnail + shorts_behaviour + livestream_behaviour )a @required_fields ~w(name output_path_template)a @@ -33,6 +35,14 @@ defmodule Pinchflat.Profiles.MediaProfile do field :download_thumbnail, :boolean, default: true field :embed_thumbnail, :boolean, default: true + # NOTE: these do NOT speed up indexing - the indexer still has to go + # through the entire collection to determine if a video is a short or + # a livestream. + # NOTE: these can BOTH be set to :only which will download shorts and + # livestreams _only_ and ignore regular videos. + field :shorts_behaviour, Ecto.Enum, values: [:include, :exclude, :only], default: :include + field :livestream_behaviour, Ecto.Enum, values: [:include, :exclude, :only], default: :include + has_many :sources, Source timestamps(type: :utc_datetime) diff --git a/lib/pinchflat/profiles/options/yt_dlp/option_builder.ex b/lib/pinchflat/profiles/options/yt_dlp/download_option_builder.ex similarity index 91% rename from lib/pinchflat/profiles/options/yt_dlp/option_builder.ex rename to lib/pinchflat/profiles/options/yt_dlp/download_option_builder.ex index 06aa9db..cbea643 100644 --- a/lib/pinchflat/profiles/options/yt_dlp/option_builder.ex +++ b/lib/pinchflat/profiles/options/yt_dlp/download_option_builder.ex @@ -1,6 +1,6 @@ -defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilder do +defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder do @moduledoc """ - Builds the options for yt-dlp based on the given media profile. + Builds the options for yt-dlp to download media based on the given media profile. IDEA: consider making this a behaviour so I can add other backends later """ @@ -9,7 +9,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilder do alias Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder @doc """ - Builds the options for yt-dlp based on the given media profile. + Builds the options for yt-dlp to download media based on the given media profile. IDEA: consider adding the ability to pass in a second argument to override these options diff --git a/lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex b/lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex new file mode 100644 index 0000000..2040271 --- /dev/null +++ b/lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex @@ -0,0 +1,52 @@ +defmodule Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder do + @moduledoc """ + Builds the options for yt-dlp to index a media source based on the given media profile. + """ + + alias Pinchflat.Profiles.MediaProfile + + @doc """ + Builds the options for yt-dlp to index a media source based on the given media profile. + """ + def build(%MediaProfile{} = media_profile) do + built_options = release_type_options(media_profile) + + {:ok, built_options} + end + + defp release_type_options(media_profile) do + mapped_struct = Map.from_struct(media_profile) + + # Appending multiple match filters treats them as an OR condition, + # so we have to be careful around combining `only` and `exclude` options. + # eg: only shorts + exclude livestreams = "any video that is a short OR is not a livestream" + # which will return all shorts AND normal videos. + Enum.reduce(mapped_struct, [], fn attr, acc -> + case {attr, media_profile} do + {{:shorts_behaviour, :only}, _} -> + acc ++ [match_filter: "original_url*=/shorts/"] + + {{:livestream_behaviour, :only}, _} -> + acc ++ [match_filter: "was_live"] + + # Since match_filter is an OR (see above), `exclude`s must be ignored entirely if the + # other type is set to `only`. There is also special behaviour if they're both excludes, + # hence why these check against `:include` alone. + {{:shorts_behaviour, :exclude}, %{livestream_behaviour: :include}} -> + acc ++ [match_filter: "original_url!*=/shorts/"] + + {{:livestream_behaviour, :exclude}, %{shorts_behaviour: :include}} -> + acc ++ [match_filter: "!was_live"] + + # Again, since it's an OR, there's a special syntax if they're both excluded + # to make it an AND. Note that I'm not checking for the other permutation of + # both excluding since this MUST get hit so adding the other version would double up. + {{:livestream_behaviour, :exclude}, %{shorts_behaviour: :exclude}} -> + acc ++ [match_filter: "!was_live & original_url!*=/shorts/"] + + _ -> + acc + end + end) + end +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 b058784..8356e88 100644 --- a/lib/pinchflat/profiles/options/yt_dlp/output_path_builder.ex +++ b/lib/pinchflat/profiles/options/yt_dlp/output_path_builder.ex @@ -12,6 +12,11 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder do Translates liquid-style templates into yt-dlp-style templates, leaving yt-dlp syntax intact. + + IDEA: apart from any custom options I've defined, I can support any yt-dlp + option by assuming `{{ identifier }}` should transform to `%(identifier)S`. + It's not doing anything huge, but it's nicer to type and more approachable IMO. + IDEA: set a default for `MediaProfile`'s `output_path_template` field """ def build(template_string) do TemplateParser.parse(template_string, full_yt_dlp_options_map()) diff --git a/priv/repo/migrations/20240205215634_add_release_type_behaviour_to_media_profiles.exs b/priv/repo/migrations/20240205215634_add_release_type_behaviour_to_media_profiles.exs new file mode 100644 index 0000000..96da10c --- /dev/null +++ b/priv/repo/migrations/20240205215634_add_release_type_behaviour_to_media_profiles.exs @@ -0,0 +1,10 @@ +defmodule Pinchflat.Repo.Migrations.AddReleaseTypeBehaviourToMediaProfiles do + use Ecto.Migration + + def change do + alter table(:media_profiles) do + add :shorts_behaviour, :string, null: false, default: "include" + add :livestream_behaviour, :string, null: false, default: "include" + end + end +end diff --git a/test/pinchflat/media_client/source_details_test.exs b/test/pinchflat/media_client/source_details_test.exs index 5d79cc9..58de272 100644 --- a/test/pinchflat/media_client/source_details_test.exs +++ b/test/pinchflat/media_client/source_details_test.exs @@ -1,6 +1,8 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do - use ExUnit.Case, async: true + use Pinchflat.DataCase import Mox + import Pinchflat.ProfilesFixtures + import Pinchflat.MediaSourceFixtures alias Pinchflat.MediaClient.SourceDetails @@ -41,7 +43,7 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do end end - describe "get_video_ids/2" do + describe "get_video_ids/2 when passed a string" do test "it passes the expected arguments to the backend" do expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot -> assert opts == [:simulate, :skip_download] @@ -61,4 +63,33 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do assert {:ok, ["video1", "video2", "video3"]} = SourceDetails.get_video_ids(@channel_url) end end + + describe "get_video_ids/2 when passed a Source record" do + test "it calls the backend with the source's collection ID" do + source = source_fixture() + + expect(YtDlpRunnerMock, :run, fn url, _opts, _ot -> + assert source.collection_id == url + {:ok, "video1\nvideo2\nvideo3"} + end) + + assert {:ok, _} = SourceDetails.get_video_ids(source) + end + + test "it builds options based on the source's media profile" do + expect(YtDlpRunnerMock, :run, fn _url, opts, _ot -> + assert opts == [{:match_filter, "!was_live"}, :simulate, :skip_download] + {:ok, ""} + end) + + media_profile = + media_profile_fixture( + shorts_behaviour: :include, + livestream_behaviour: :exclude + ) + + source = source_fixture(media_profile_id: media_profile.id) + assert {:ok, _} = SourceDetails.get_video_ids(source) + end + end end diff --git a/test/pinchflat/profiles/options/yt_dlp/option_builder_test.exs b/test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs similarity index 74% rename from test/pinchflat/profiles/options/yt_dlp/option_builder_test.exs rename to test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs index 30982b9..7dc9904 100644 --- a/test/pinchflat/profiles/options/yt_dlp/option_builder_test.exs +++ b/test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs @@ -1,8 +1,8 @@ -defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do +defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do use ExUnit.Case, async: true alias Pinchflat.Profiles.MediaProfile - alias Pinchflat.Profiles.Options.YtDlp.OptionBuilder + alias Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder @media_profile %MediaProfile{ output_path_template: "{{ title }}.%(ext)s" @@ -10,7 +10,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do describe "build/1" do test "it generates an expanded output path based on the given template" do - assert {:ok, res} = OptionBuilder.build(@media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(@media_profile) assert {:output, "/tmp/videos/%(title)S.%(ext)s"} in res end @@ -20,7 +20,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest 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 {:ok, res} = DownloadOptionBuilder.build(media_profile) assert :write_subs in res end @@ -28,7 +28,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do 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 {:ok, res} = DownloadOptionBuilder.build(media_profile) assert {:convert_subs, "srt"} in res end @@ -36,7 +36,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do 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 {:ok, res} = DownloadOptionBuilder.build(media_profile) assert :write_auto_subs in res end @@ -44,7 +44,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do 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) + assert {:ok, res} = DownloadOptionBuilder.build(media_profile) refute :write_auto_subs in res end @@ -52,7 +52,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do test "includes :embed_subs option when specified" do media_profile = %MediaProfile{@media_profile | embed_subs: true} - assert {:ok, res} = OptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_profile) assert :embed_subs in res end @@ -60,7 +60,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do 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 {:ok, res} = DownloadOptionBuilder.build(media_profile) assert {:sub_langs, "en"} in res end @@ -68,7 +68,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do 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 {:ok, res} = DownloadOptionBuilder.build(media_profile) assert {:sub_langs, "en"} in res end @@ -81,7 +81,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do sub_langs: "en" } - assert {:ok, res} = OptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_profile) refute {:sub_langs, "en"} in res end @@ -89,7 +89,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do test "other struct attributes are ignored" do media_profile = %MediaProfile{@media_profile | id: -1} - assert {:ok, res} = OptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_profile) refute {:id, -1} in res end @@ -99,7 +99,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do test "includes :write_thumbnail option when specified" do media_profile = %MediaProfile{@media_profile | download_thumbnail: true} - assert {:ok, res} = OptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_profile) assert :write_thumbnail in res end @@ -107,7 +107,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do test "includes :embed_thumbnail option when specified" do media_profile = %MediaProfile{@media_profile | embed_thumbnail: true} - assert {:ok, res} = OptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_profile) assert :embed_thumbnail in res end @@ -119,7 +119,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do download_thumbnail: false } - assert {:ok, res} = OptionBuilder.build(media_profile) + assert {:ok, res} = DownloadOptionBuilder.build(media_profile) refute :write_thumbnail in res refute :embed_thumbnail in res diff --git a/test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs b/test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs new file mode 100644 index 0000000..07c315d --- /dev/null +++ b/test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs @@ -0,0 +1,104 @@ +defmodule Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilderTest do + use ExUnit.Case, async: true + + alias Pinchflat.Profiles.MediaProfile + alias Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder + + @media_profile %MediaProfile{ + output_path_template: "{{ title }}.%(ext)s", + shorts_behaviour: :include, + livestream_behaviour: :include + } + + describe "build/1 when testing release type options" do + test "adds correct filter when shorts_behaviour is :only" do + media_profile = %MediaProfile{@media_profile | shorts_behaviour: :only} + + assert {:ok, res} = IndexOptionBuilder.build(media_profile) + + assert {:match_filter, "original_url*=/shorts/"} in res + refute {:match_filter, "original_url!*=/shorts/"} in res + refute {:match_filter, "!was_live"} in res + refute {:match_filter, "was_live"} in res + end + + test "adds correct filter when livestream_behaviour is :only" do + media_profile = %MediaProfile{@media_profile | livestream_behaviour: :only} + + assert {:ok, res} = IndexOptionBuilder.build(media_profile) + + assert {:match_filter, "was_live"} in res + refute {:match_filter, "!was_live"} in res + refute {:match_filter, "!original_url*=/shorts/"} in res + refute {:match_filter, "original_url*=/shorts/"} in res + end + + test "adds correct filter when both livestreams and shorts are :only" do + media_profile = %MediaProfile{ + @media_profile + | shorts_behaviour: :only, + livestream_behaviour: :only + } + + assert {:ok, res} = IndexOptionBuilder.build(media_profile) + + assert {:match_filter, "original_url*=/shorts/"} in res + assert {:match_filter, "was_live"} in res + refute {:match_filter, "original_url!*=/shorts/"} in res + refute {:match_filter, "!was_live"} in res + end + + test "adds correct filter when shorts_behaviour is :exclude" do + media_profile = %MediaProfile{@media_profile | shorts_behaviour: :exclude} + + assert {:ok, res} = IndexOptionBuilder.build(media_profile) + + assert {:match_filter, "original_url!*=/shorts/"} in res + refute {:match_filter, "original_url*=/shorts/"} in res + refute {:match_filter, "was_live"} in res + refute {:match_filter, "!was_live"} in res + end + + test "adds correct filter when livestream_behaviour is :exclude" do + media_profile = %MediaProfile{@media_profile | livestream_behaviour: :exclude} + + assert {:ok, res} = IndexOptionBuilder.build(media_profile) + + assert {:match_filter, "!was_live"} in res + refute {:match_filter, "was_live"} in res + refute {:match_filter, "original_url!*=/shorts/"} in res + refute {:match_filter, "original_url*=/shorts/"} in res + end + + test "adds correct filter when shorts and livestreams are both exclude" do + media_profile = %MediaProfile{ + @media_profile + | shorts_behaviour: :exclude, + livestream_behaviour: :exclude + } + + assert {:ok, res} = IndexOptionBuilder.build(media_profile) + + assert {:match_filter, "!was_live & original_url!*=/shorts/"} in res + refute {:match_filter, "original_url!*=/shorts/"} in res + refute {:match_filter, "!was_live"} in res + refute {:match_filter, "original_url*=/shorts/"} in res + refute {:match_filter, "was_live"} in res + end + + test "does not add exclusion filter if one is excluded and the other is only" do + media_profile = %MediaProfile{ + @media_profile + | shorts_behaviour: :exclude, + livestream_behaviour: :only + } + + assert {:ok, res} = IndexOptionBuilder.build(media_profile) + + assert {:match_filter, "was_live"} in res + refute {:match_filter, "original_url!*=/shorts/"} in res + refute {:match_filter, "original_url*=/shorts/"} in res + refute {:match_filter, "!was_live"} in res + end + end +end