Improve short detection (#59)

* Made media attribute-related yt-dlp calls return a struct

* Added shorts attribute to media items

* Added ability to discern a short from yt-dlp response

* Updated search to use new shorts attribute

lib/pinchflat/media.ex

@@ -188,17 +188,10 @@ defmodule Pinchflat.Media do
 
   Returns {:ok, %MediaItem{}} | {:error, %Ecto.Changeset{}}
   """
-  def create_media_item_from_backend_attrs(source, media_attrs) do
+  def create_media_item_from_backend_attrs(source, media_attrs_struct) do
-    attrs = %{
+    %{source_id: source.id}
-      source_id: source.id,
+    |> Map.merge(Map.from_struct(media_attrs_struct))
-      title: media_attrs["title"],
+    |> create_media_item()
-      media_id: media_attrs["id"],
-      original_url: media_attrs["original_url"],
-      livestream: media_attrs["was_live"],
-      description: media_attrs["description"]
-    }
-
-    create_media_item(attrs)
   end
 
   @doc """
@@ -264,7 +257,7 @@ defmodule Pinchflat.Media do
         {{:shorts_behaviour, :only}, %{livestream_behaviour: :only}} ->
           dynamic(
             [mi],
-            ^dynamic and (mi.livestream == true or fragment("LOWER(?) LIKE LOWER(?)", mi.original_url, "%/shorts/%"))
+            ^dynamic and (mi.livestream == true or mi.short_form_content == true)
           )
 
         # Technically redundant, but makes the other clauses easier to parse
@@ -273,16 +266,13 @@ defmodule Pinchflat.Media do
           dynamic
 
         {{:shorts_behaviour, :only}, _} ->
-          # return records with /shorts/ in the original_url
+          dynamic([mi], ^dynamic and mi.short_form_content == true)
-          dynamic([mi], ^dynamic and fragment("LOWER(?) LIKE LOWER(?)", mi.original_url, "%/shorts/%"))
 
         {{:livestream_behaviour, :only}, _} ->
-          # return records with livestream: true
           dynamic([mi], ^dynamic and mi.livestream == true)
 
         {{:shorts_behaviour, :exclude}, %{livestream_behaviour: lb}} when lb != :only ->
-          # return records without /shorts/ in the original_url
+          dynamic([mi], ^dynamic and mi.short_form_content == false)
-          dynamic([mi], ^dynamic and fragment("LOWER(?) NOT LIKE LOWER(?)", mi.original_url, "%/shorts/%"))
 
         {{:livestream_behaviour, :exclude}, %{shorts_behaviour: sb}} when sb != :only ->
           # return records with livestream: false

lib/pinchflat/media/media_item.ex

@@ -19,6 +19,7 @@ defmodule Pinchflat.Media.MediaItem do
     :original_url,
     :livestream,
     :source_id,
+    :short_form_content,
     # these fields are captured only on download
     :media_downloaded_at,
     :media_filepath,
@@ -27,7 +28,7 @@ defmodule Pinchflat.Media.MediaItem do
     :thumbnail_filepath,
     :metadata_filepath
   ]
-  @required_fields ~w(title original_url livestream media_id source_id)a
+  @required_fields ~w(title original_url livestream media_id source_id short_form_content)a
 
   schema "media_items" do
     field :title, :string
@@ -35,6 +36,7 @@ defmodule Pinchflat.Media.MediaItem do
     field :description, :string
     field :original_url, :string
     field :livestream, :boolean, default: false
+    field :short_form_content, :boolean, default: false
     field :media_downloaded_at, :utc_datetime
 
     field :media_filepath, :string

lib/pinchflat/tasks/source_tasks.ex

@@ -20,6 +20,8 @@ defmodule Pinchflat.Tasks.SourceTasks do
   alias Pinchflat.Workers.MediaCollectionIndexingWorker
   alias Pinchflat.Utils.FilesystemUtils.FileFollowerServer
 
+  alias Pinchflat.YtDlp.Backend.Media, as: YtDlpMedia
+
   @doc """
   Starts tasks for indexing a source's media regardless of the source's indexing
   frequency. It's assumed the caller will check for that.
@@ -162,7 +164,8 @@ defmodule Pinchflat.Tasks.SourceTasks do
         {:ok, media_attrs} ->
           Logger.debug("FileFollowerServer Handler: Got media attributes: #{inspect(media_attrs)}")
 
-          create_media_item_and_enqueue_download(source, media_attrs)
+          media_struct = YtDlpMedia.response_to_struct(media_attrs)
+          create_media_item_and_enqueue_download(source, media_struct)
 
         err ->
           Logger.debug("FileFollowerServer Handler: Error decoding JSON: #{inspect(err)}")

lib/pinchflat/yt_dlp/backend/media.ex

@@ -3,6 +3,27 @@ defmodule Pinchflat.YtDlp.Backend.Media do
   Contains utilities for working with singular pieces of media
   """
 
+  @enforce_keys [
+    :media_id,
+    :title,
+    :description,
+    :original_url,
+    :livestream,
+    :short_form_content
+  ]
+
+  defstruct [
+    :media_id,
+    :title,
+    :description,
+    :original_url,
+    :livestream,
+    :short_form_content
+  ]
+
+  alias __MODULE__
+  alias Pinchflat.Utils.FunctionUtils
+
   @doc """
   Downloads a single piece of media (and possibly its metadata) directly to its
   final destination. Returns the parsed JSON output from yt-dlp.
@@ -23,11 +44,6 @@ defmodule Pinchflat.YtDlp.Backend.Media do
   @doc """
   Returns a map representing the media at the given URL.
 
-  IDEA: should I return a struct here? I want these methods to be agnostic
-  to implementation, but maybe it can specify its own contract by
-  returning a struct with well-known fields. Would make refactoring
-  turbo easy if a field needs to exist but its behavior changes slightly.
-
   Returns {:ok, [map()]} | {:error, any, ...}.
   """
   def get_media_attributes(url) do
@@ -36,8 +52,14 @@ defmodule Pinchflat.YtDlp.Backend.Media do
     output_template = indexing_output_template()
 
     case runner.run(url, command_opts, output_template) do
-      {:ok, output} -> Phoenix.json_library().decode(output)
+      {:ok, output} ->
-      res -> res
+        output
+        |> Phoenix.json_library().decode!()
+        |> response_to_struct()
+        |> FunctionUtils.wrap_ok()
+
+      res ->
+        res
     end
   end
 
@@ -45,7 +67,37 @@ defmodule Pinchflat.YtDlp.Backend.Media do
   Returns the output template for yt-dlp's indexing command.
   """
   def indexing_output_template do
-    "%(.{id,title,was_live,original_url,description})j"
+    "%(.{id,title,was_live,webpage_url,description,aspect_ratio,duration})j"
+  end
+
+  @doc """
+  Transforms a response from yt-dlp into a struct. Interprets the response to
+  determine if the media is short-form content.
+
+  Returns %Media{}.
+  """
+  def response_to_struct(response) do
+    %Media{
+      media_id: response["id"],
+      title: response["title"],
+      description: response["description"],
+      original_url: response["webpage_url"],
+      livestream: response["was_live"],
+      short_form_content: short_form_content?(response)
+    }
+  end
+
+  defp short_form_content?(response) do
+    if String.contains?(response["webpage_url"], "/shorts/") do
+      true
+    else
+      # Sometimes shorts are returned without /shorts/ in the URL,
+      # so we need to do our best to determine if it's a short. This
+      # WILL returns false positives, but it's a best-effort approach
+      # that should work for most cases. The aspect_ratio check is
+      # based on a gut feeling and may need to be tweaked.
+      response["duration"] <= 60 && response["aspect_ratio"] < 0.8
+    end
   end
 
   defp backend_runner do

lib/pinchflat/yt_dlp/backend/media_collection.ex

@@ -36,6 +36,7 @@ defmodule Pinchflat.YtDlp.Backend.MediaCollection do
         output
         |> String.split("\n", trim: true)
         |> Enum.map(&Phoenix.json_library().decode!/1)
+        |> Enum.map(&YtDlpMedia.response_to_struct/1)
         |> FunctionUtils.wrap_ok()
 
       res ->

priv/repo/migrations/20240309052602_add_short_form_to_media_items.exs

@@ -0,0 +1,9 @@
+defmodule Pinchflat.Repo.Migrations.AddShortFormToMediaItems do
+  use Ecto.Migration
+
+  def change do
+    alter table(:media_items) do
+      add :short_form_content, :boolean, null: false, default: false
+    end
+  end
+end

test/pinchflat/media_test.exs

@@ -11,6 +11,8 @@ defmodule Pinchflat.MediaTest do
   alias Pinchflat.Media.MediaItem
   alias Pinchflat.Metadata.MetadataFileHelpers
 
+  alias Pinchflat.YtDlp.Backend.Media, as: YtDlpMedia
+
   setup :verify_on_exit!
 
   @invalid_attrs %{title: nil, media_id: nil, media_filepath: nil}
@@ -95,7 +97,7 @@ defmodule Pinchflat.MediaTest do
     test "returns shorts and normal media when shorts_behaviour is :include" do
       source = source_fixture(%{media_profile_id: media_profile_fixture(%{shorts_behaviour: :include}).id})
       normal = media_item_fixture(%{source_id: source.id, media_filepath: nil})
-      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       assert Media.list_pending_media_items_for(source) == [normal, short]
     end
@@ -103,7 +105,7 @@ defmodule Pinchflat.MediaTest do
     test "returns only shorts when shorts_behaviour is :only" do
       source = source_fixture(%{media_profile_id: media_profile_fixture(%{shorts_behaviour: :only}).id})
       _normal = media_item_fixture(%{source_id: source.id, media_filepath: nil})
-      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       assert Media.list_pending_media_items_for(source) == [short]
     end
@@ -111,7 +113,7 @@ defmodule Pinchflat.MediaTest do
     test "returns only normal media when shorts_behaviour is :exclude" do
       source = source_fixture(%{media_profile_id: media_profile_fixture(%{shorts_behaviour: :exclude}).id})
       normal = media_item_fixture(%{source_id: source.id, media_filepath: nil})
-      _short = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      _short = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       assert Media.list_pending_media_items_for(source) == [normal]
     end
@@ -156,7 +158,7 @@ defmodule Pinchflat.MediaTest do
 
       normal = media_item_fixture(%{source_id: source.id, media_filepath: nil})
       livestream = media_item_fixture(%{source_id: source.id, media_filepath: nil, livestream: true})
-      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       assert Media.list_pending_media_items_for(source) == [normal, livestream, short]
     end
@@ -173,7 +175,7 @@ defmodule Pinchflat.MediaTest do
 
       _normal = media_item_fixture(%{source_id: source.id, media_filepath: nil})
       livestream = media_item_fixture(%{source_id: source.id, media_filepath: nil, livestream: true})
-      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       assert Media.list_pending_media_items_for(source) == [livestream, short]
     end
@@ -190,7 +192,7 @@ defmodule Pinchflat.MediaTest do
 
       normal = media_item_fixture(%{source_id: source.id, media_filepath: nil})
       _livestream = media_item_fixture(%{source_id: source.id, media_filepath: nil, livestream: true})
-      _short = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      _short = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       assert Media.list_pending_media_items_for(source) == [normal]
     end
@@ -207,7 +209,7 @@ defmodule Pinchflat.MediaTest do
 
       _normal = media_item_fixture(%{source_id: source.id, media_filepath: nil})
       _livestream = media_item_fixture(%{source_id: source.id, media_filepath: nil, livestream: true})
-      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      short = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       assert Media.list_pending_media_items_for(source) == [short]
     end
@@ -247,7 +249,7 @@ defmodule Pinchflat.MediaTest do
 
     test "returns false if the media hasn't been downloaded but the profile doesn't DL shorts" do
       source = source_fixture(%{media_profile_id: media_profile_fixture(%{shorts_behaviour: :exclude}).id})
-      media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil, original_url: "/shorts/"})
+      media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil, short_form_content: true})
 
       refute Media.pending_download?(media_item)
     end
@@ -388,14 +390,18 @@ defmodule Pinchflat.MediaTest do
   describe "create_media_item_from_backend_attrs/2" do
     test "creates a media item for a given source and attributes" do
       source = source_fixture()
-      media_attrs = Phoenix.json_library().decode!(media_attributes_return_fixture())
+
+      media_attrs =
+        media_attributes_return_fixture()
+        |> Phoenix.json_library().decode!()
+        |> YtDlpMedia.response_to_struct()
 
       assert {:ok, %MediaItem{} = media_item} = Media.create_media_item_from_backend_attrs(source, media_attrs)
       assert media_item.source_id == source.id
-      assert media_item.title == media_attrs["title"]
+      assert media_item.title == media_attrs.title
-      assert media_item.media_id == media_attrs["id"]
+      assert media_item.media_id == media_attrs.media_id
-      assert media_item.original_url == media_attrs["original_url"]
+      assert media_item.original_url == media_attrs.original_url
-      assert media_item.description == media_attrs["description"]
+      assert media_item.description == media_attrs.description
     end
   end
 

test/pinchflat/tasks/media_items_tasks_test.exs

@@ -84,9 +84,11 @@ defmodule Pinchflat.Tasks.MediaItemTasksTest do
           Phoenix.json_library().encode!(%{
             id: "video2",
             title: "Video 2",
-            original_url: "https://example.com/shorts/video2",
+            webpage_url: "https://example.com/shorts/video2",
             was_live: true,
-            description: "desc2"
+            description: "desc2",
+            aspect_ratio: 1.67,
+            duration: 345.67
           })
 
         {:ok, output}

test/pinchflat/tasks/source_tasks_test.exs

@@ -229,9 +229,11 @@ defmodule Pinchflat.Tasks.SourceTasksTest do
           Phoenix.json_library().encode!(%{
             id: "video2",
             title: "Video 2",
-            original_url: "https://example.com/shorts/video2",
+            webpage_url: "https://example.com/shorts/video2",
             was_live: true,
-            description: "desc2"
+            description: "desc2",
+            aspect_ratio: 1.67,
+            duration: 345.67
           })
 
         File.write(filepath, contents)

test/pinchflat/yt_dlp/backend/media_collection_test.exs

@@ -16,7 +16,7 @@ defmodule Pinchflat.YtDlp.Backend.MediaCollectionTest do
         {:ok, source_attributes_return_fixture() <> "\n\n"}
       end)
 
-      assert {:ok, [%{"id" => "video1"}, %{"id" => "video2"}, %{"id" => "video3"}]} =
+      assert {:ok, [%Media{media_id: "video1"}, %Media{media_id: "video2"}, %Media{media_id: "video3"}]} =
                MediaCollection.get_media_attributes_for_collection(@channel_url)
     end
 

test/pinchflat/yt_dlp/backend/media_test.exs

@@ -55,7 +55,7 @@ defmodule Pinchflat.YtDlp.Backend.MediaTest do
         {:ok, media_attributes_return_fixture()}
       end)
 
-      assert {:ok, %{"description" => _, "id" => _, "original_url" => _, "title" => _, "was_live" => _}} =
+      assert {:ok, %{description: _, media_id: _, original_url: _, title: _, livestream: _}} =
                Media.get_media_attributes(@media_url)
     end
 
@@ -64,7 +64,7 @@ defmodule Pinchflat.YtDlp.Backend.MediaTest do
         assert opts == [:simulate, :skip_download]
         assert ot == Media.indexing_output_template()
 
-        {:ok, "{}"}
+        {:ok, media_attributes_return_fixture()}
       end)
 
       assert {:ok, _} = Media.get_media_attributes(@media_url)
@@ -79,8 +79,61 @@ defmodule Pinchflat.YtDlp.Backend.MediaTest do
 
   describe "indexing_output_template/0" do
     test "contains all the greatest hits" do
-      assert "%(.{id,title,was_live,original_url,description})j" ==
+      assert "%(.{id,title,was_live,webpage_url,description,aspect_ratio,duration})j" ==
                Media.indexing_output_template()
     end
   end
+
+  describe "response_to_struct/1" do
+    test "transforms a response into a struct" do
+      response = %{
+        "id" => "TiZPUDkDYbk",
+        "title" => "Trying to Wheelie Without the Rear Brake",
+        "description" => "I'm not sure what I expected.",
+        "webpage_url" => "https://www.youtube.com/watch?v=TiZPUDkDYbk",
+        "was_live" => false,
+        "aspect_ratio" => 1.0,
+        "duration" => 60
+      }
+
+      assert %Media{
+               media_id: "TiZPUDkDYbk",
+               title: "Trying to Wheelie Without the Rear Brake",
+               description: "I'm not sure what I expected.",
+               original_url: "https://www.youtube.com/watch?v=TiZPUDkDYbk",
+               livestream: false,
+               short_form_content: false
+             } = Media.response_to_struct(response)
+    end
+
+    test "sets short_form_content to true if the URL contains /shorts/" do
+      response = %{
+        "webpage_url" => "https://www.youtube.com/shorts/TiZPUDkDYbk",
+        "aspect_ratio" => 1.0,
+        "duration" => 61
+      }
+
+      assert %Media{short_form_content: true} = Media.response_to_struct(response)
+    end
+
+    test "sets short_form_content to true if the aspect ratio are duration are right" do
+      response = %{
+        "webpage_url" => "https://www.youtube.com/watch?v=TiZPUDkDYbk",
+        "aspect_ratio" => 0.5,
+        "duration" => 59
+      }
+
+      assert %Media{short_form_content: true} = Media.response_to_struct(response)
+    end
+
+    test "sets short_form_content to false otherwise" do
+      response = %{
+        "webpage_url" => "https://www.youtube.com/watch?v=TiZPUDkDYbk",
+        "aspect_ratio" => 1.0,
+        "duration" => 61
+      }
+
+      assert %Media{short_form_content: false} = Media.response_to_struct(response)
+    end
+  end
 end

test/support/fixtures/media_fixtures.ex

@@ -19,6 +19,7 @@ defmodule Pinchflat.MediaFixtures do
         title: Faker.Commerce.product_name(),
         original_url: "https://www.youtube.com/watch?v=#{media_id}",
         livestream: false,
+        short_form_content: false,
         media_filepath: "/video/#{Faker.File.file_name(:video)}",
         source_id: SourcesFixtures.source_fixture().id
       })
@@ -70,9 +71,11 @@ defmodule Pinchflat.MediaFixtures do
     media_attributes = %{
       id: "video1",
       title: "Video 1",
-      original_url: "https://example.com/video1",
+      webpage_url: "https://example.com/video1",
       was_live: false,
-      description: "desc1"
+      description: "desc1",
+      aspect_ratio: 1.67,
+      duration: 123.45
     }
 
     Phoenix.json_library().encode!(media_attributes)

test/support/fixtures/sources_fixtures.ex

@@ -35,23 +35,29 @@ defmodule Pinchflat.SourcesFixtures do
       %{
         id: "video1",
         title: "Video 1",
-        original_url: "https://example.com/video1",
+        webpage_url: "https://example.com/video1",
         was_live: false,
-        description: "desc1"
+        description: "desc1",
+        aspect_ratio: 1.67,
+        duration: 12.34
       },
       %{
         id: "video2",
         title: "Video 2",
-        original_url: "https://example.com/video2",
+        webpage_url: "https://example.com/video2",
         was_live: true,
-        description: "desc2"
+        description: "desc2",
+        aspect_ratio: 1.67,
+        duration: 345.67
       },
       %{
         id: "video3",
         title: "Video 3",
-        original_url: "https://example.com/video3",
+        webpage_url: "https://example.com/video3",
         was_live: false,
-        description: "desc3"
+        description: "desc3",
+        aspect_ratio: 1.0,
+        duration: 678.90
       }
     ]