Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bugfix] Ensure livestreams aren't downloaded until they're finished processing #485

Merged
merged 4 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/pinchflat/downloading/media_download_worker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
{:recovered, _} ->
{:error, :retry}

{:error, :unsuitable_for_download} ->
{:ok, :non_retry}

{:error, message} ->
action_on_error(message)
end
Expand Down
13 changes: 12 additions & 1 deletion lib/pinchflat/downloading/media_downloader.ex
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ defmodule Pinchflat.Downloading.MediaDownloader do
{:ok, parsed_json} ->
update_media_item_from_parsed_json(media_with_preloads, parsed_json)

{:error, :unsuitable_for_download} ->
Logger.warning(
"Media item ##{media_with_preloads.id} isn't suitable for download yet. May be an active or processing live stream"
)

{:error, :unsuitable_for_download}

{:error, message, _exit_code} ->
Logger.error("yt-dlp download error for media item ##{media_with_preloads.id}: #{inspect(message)}")

Expand Down Expand Up @@ -108,7 +115,11 @@ defmodule Pinchflat.Downloading.MediaDownloader do
{:ok, options} = DownloadOptionBuilder.build(item_with_preloads, override_opts)
runner_opts = [output_filepath: output_filepath, use_cookies: item_with_preloads.source.use_cookies]

YtDlpMedia.download(url, options, runner_opts)
case YtDlpMedia.get_downloadable_status(url) do
{:ok, :downloadable} -> YtDlpMedia.download(url, options, runner_opts)
{:ok, :ignorable} -> {:error, :unsuitable_for_download}
err -> err
end
end

defp recoverable_errors do
Expand Down
31 changes: 29 additions & 2 deletions lib/pinchflat/yt_dlp/media.ex
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,24 @@ defmodule Pinchflat.YtDlp.Media do
end
end

@doc """
Determines if the media at the given URL is ready to be downloaded.
Common examples of non-downloadable media are upcoming or in-progress live streams.

Returns {:ok, :downloadable | :ignorable} | {:error, any}
"""
def get_downloadable_status(url) do
case backend_runner().run(url, [:simulate, :skip_download], "%(.{live_status})j") do
{:ok, output} ->
output
|> Phoenix.json_library().decode!()
|> parse_downloadable_status()

err ->
err
end
end

@doc """
Downloads a thumbnail for a single piece of media. Usually used for
downloading thumbnails for internal use
Expand All @@ -71,11 +89,10 @@ defmodule Pinchflat.YtDlp.Media do
Returns {:ok, %Media{}} | {:error, any, ...}.
"""
def get_media_attributes(url, command_opts \\ [], addl_opts \\ []) do
runner = Application.get_env(:pinchflat, :yt_dlp_runner)
all_command_opts = [:simulate, :skip_download] ++ command_opts
output_template = indexing_output_template()

case runner.run(url, all_command_opts, output_template, addl_opts) do
case backend_runner().run(url, all_command_opts, output_template, addl_opts) do
{:ok, output} ->
output
|> Phoenix.json_library().decode!()
Expand Down Expand Up @@ -147,6 +164,16 @@ defmodule Pinchflat.YtDlp.Media do
defp parse_uploaded_at(%{"upload_date" => nil}), do: nil
defp parse_uploaded_at(response), do: MetadataFileHelpers.parse_upload_date(response["upload_date"])

defp parse_downloadable_status(response) do
case response["live_status"] do
status when status in ["is_live", "is_upcoming", "post_live"] -> {:ok, :ignorable}
status when status in ["was_live", "not_live"] -> {:ok, :downloadable}
# This preserves my tenuous support for non-youtube sources.
nil -> {:ok, :downloadable}
_ -> {:error, "Unknown live status: #{response["live_status"]}"}
end
end

defp backend_runner do
# This approach lets us mock the command for testing
Application.get_env(:pinchflat, :yt_dlp_runner)
Expand Down
15 changes: 15 additions & 0 deletions test/pinchflat/downloading/media_download_worker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end)
stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
Expand Down Expand Up @@ -186,6 +187,20 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end
end

describe "perform/1 when testing non-downloadable media" do
test "does not retry the job if the media is currently not downloadable", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)

Oban.Testing.with_testing_mode(:inline, fn ->
{:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id}))

assert job.state == "completed"
end)
end
end

describe "perform/1 when testing forced downloads" do
test "ignores 'prevent_download' if forced", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> :ok end)
Expand Down
39 changes: 39 additions & 0 deletions test/pinchflat/downloading/media_downloader_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
)

stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end)
stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_args -> {:ok, ""} end)

{:ok, %{media_item: media_item}}
Expand Down Expand Up @@ -49,6 +50,14 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
assert updated_media_item.metadata.thumbnail_filepath =~ "media_items/#{media_item.id}/thumbnail.jpg"
end

test "errors for non-downloadable media are passed through", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)

assert {:error, :unsuitable_for_download} = MediaDownloader.download_for_media_item(media_item)
end

test "non-recoverable errors are passed through", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
{:error, :some_error, 1}
Expand All @@ -67,6 +76,36 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
end
end

describe "download_for_media_item/3 when testing non-downloadable media" do
test "calls the download runner if the media is currently downloadable", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "was_live"})}
end)

expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot, _addl ->
{:ok, render_metadata(:media_metadata)}
end)

assert {:ok, _} = MediaDownloader.download_for_media_item(media_item)
end

test "does not call the download runner if the media is not downloadable", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)

expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot, _addl -> {:ok, ""} end)

assert {:error, :unsuitable_for_download} = MediaDownloader.download_for_media_item(media_item)
end

test "returns unexpected errors from the download status determination method", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, :what_tha} end)

assert {:error, "Unknown error: {:error, :what_tha}"} = MediaDownloader.download_for_media_item(media_item)
end
end

describe "download_for_media_item/3 when testing override options" do
test "includes override opts if specified", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, opts, _ot, _addl ->
Expand Down
58 changes: 58 additions & 0 deletions test/pinchflat/yt_dlp/media_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,64 @@ defmodule Pinchflat.YtDlp.MediaTest do
end
end

describe "get_downloadable_status/1" do
test "returns :downloadable if the media was never live" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "not_live"})}
end)

assert {:ok, :downloadable} = Media.get_downloadable_status(@media_url)
end

test "returns :downloadable if the media was live and has been processed" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "was_live"})}
end)

assert {:ok, :downloadable} = Media.get_downloadable_status(@media_url)
end

test "returns :downloadable if the media's live_status is nil" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => nil})}
end)

assert {:ok, :downloadable} = Media.get_downloadable_status(@media_url)
end

test "returns :ignorable if the media is currently live" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)

assert {:ok, :ignorable} = Media.get_downloadable_status(@media_url)
end

test "returns :ignorable if the media is scheduled to be live" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_upcoming"})}
end)

assert {:ok, :ignorable} = Media.get_downloadable_status(@media_url)
end

test "returns :ignorable if the media was live but hasn't been processed" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "post_live"})}
end)

assert {:ok, :ignorable} = Media.get_downloadable_status(@media_url)
end

test "returns an error if the downloadable status can't be determined" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "what_tha"})}
end)

assert {:error, "Unknown live status: what_tha"} = Media.get_downloadable_status(@media_url)
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, _addl ->
Expand Down