[Bugfix] Partially revert custom codec selection (#279)

* Removed and re-added codec preference columns

* Removed custom codec work from download options builder

* Updated settings UI

* Made codec preferences non-optional fields

lib/pinchflat/downloading/codec_parser.ex

@@ -1,82 +0,0 @@
-defmodule Pinchflat.Downloading.CodecParser do
-  @moduledoc """
-  Functions for generating yt-dlp codec strings
-  """
-
-  alias Pinchflat.Settings
-
-  @doc """
-  Generate a video codec string based on the value of the video_codec_preference setting.
-
-  Returns binary()
-  """
-  def generate_vcodec_string_from_settings do
-    generate_vcodec_string(Settings.get!(:video_codec_preference))
-  end
-
-  @doc """
-  Generate an audio codec string based on the value of the audio_codec_preference setting.
-
-  Returns binary()
-  """
-  def generate_acodec_string_from_settings do
-    generate_acodec_string(Settings.get!(:audio_codec_preference))
-  end
-
-  @doc """
-  Generate a video codec string from a list of video codecs.
-
-  If the list is nil or empty, the default video codec is AVC.
-
-  Returns binary()
-  """
-  def generate_vcodec_string(nil), do: "bestvideo[vcodec~='^avc']/bestvideo"
-  def generate_vcodec_string([]), do: generate_vcodec_string(nil)
-
-  def generate_vcodec_string(video_codecs) do
-    video_codecs
-    |> Enum.map(&video_codec_map()[&1])
-    |> Enum.reject(&is_nil/1)
-    |> Enum.map(&"bestvideo[vcodec~='^#{&1}']")
-    |> Enum.concat(["bestvideo"])
-    |> Enum.join("/")
-  end
-
-  @doc """
-  Generate an audio codec string from a list of audio codecs.
-
-  If the list is nil or empty, the default audio codec is MP4A.
-
-  Returns binary()
-  """
-  def generate_acodec_string(nil), do: "bestaudio[acodec~='^mp4a']/bestaudio"
-  def generate_acodec_string([]), do: generate_acodec_string(nil)
-
-  def generate_acodec_string(audio_codecs) do
-    audio_codecs
-    |> Enum.map(&audio_codec_map()[&1])
-    |> Enum.reject(&is_nil/1)
-    |> Enum.map(&"bestaudio[acodec~='^#{&1}']")
-    |> Enum.concat(["bestaudio"])
-    |> Enum.join("/")
-  end
-
-  @doc false
-  def video_codec_map do
-    %{
-      "av01" => "av01",
-      "avc" => "avc",
-      "vp9" => "vp0?9"
-    }
-  end
-
-  @doc false
-  def audio_codec_map do
-    %{
-      "aac" => "aac",
-      "mp4a" => "mp4a",
-      "mp3" => "mp3",
-      "opus" => "opus"
-    }
-  end
-end

lib/pinchflat/downloading/download_option_builder.ex

@@ -4,9 +4,9 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
   """
 
   alias Pinchflat.Sources
+  alias Pinchflat.Settings
   alias Pinchflat.Sources.Source
   alias Pinchflat.Media.MediaItem
-  alias Pinchflat.Downloading.CodecParser
   alias Pinchflat.Downloading.OutputPathBuilder
 
   alias Pinchflat.Utils.FilesystemUtils, as: FSUtils
@@ -122,13 +122,13 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
   end
 
   defp quality_options(media_profile) do
-    vcodec_string = CodecParser.generate_vcodec_string_from_settings()
-    acodec_string = CodecParser.generate_acodec_string_from_settings()
+    vcodec = Settings.get!(:video_codec_preference)
+    acodec = Settings.get!(:audio_codec_preference)
 
     case media_profile.preferred_resolution do
       # Also be aware that :audio disabled all embedding options for subtitles
       :audio ->
-        [:extract_audio, format: "#{acodec_string}/best"]
+        [:extract_audio, format_sort: "+acodec:#{acodec}"]
 
       resolution_atom ->
         {resolution_string, _} =
@@ -137,10 +137,9 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
           |> Integer.parse()
 
         [
-          format_sort: "res:#{resolution_string}",
           # Since Plex doesn't support reading metadata from MKV
           remux_video: "mp4",
-          format: "((#{vcodec_string})+(#{acodec_string}))/best"
+          format_sort: "res:#{resolution_string},+codec:#{vcodec}:#{acodec}"
         ]
     end
   end

lib/pinchflat/settings/setting.ex

@@ -16,14 +16,11 @@ defmodule Pinchflat.Settings.Setting do
     :audio_codec_preference
   ]
 
-  @virtual_fields [
-    :video_codec_preference_string,
-    :audio_codec_preference_string
-  ]
-
   @required_fields ~w(
     onboarding
     pro_enabled
+    video_codec_preference
+    audio_codec_preference
   )a
 
   schema "settings" do
@@ -33,42 +30,14 @@ defmodule Pinchflat.Settings.Setting do
     field :apprise_version, :string
     field :apprise_server, :string
 
-    field :video_codec_preference, {:array, :string}, default: []
-    field :audio_codec_preference, {:array, :string}, default: []
-    field :video_codec_preference_string, :string, default: nil, virtual: true
-    field :audio_codec_preference_string, :string, default: nil, virtual: true
+    field :video_codec_preference, :string
+    field :audio_codec_preference, :string
   end
 
   @doc false
   def changeset(setting, attrs) do
     setting
     |> cast(attrs, @allowed_fields)
-    |> cast(attrs, @virtual_fields, empty_values: [])
-    |> convert_codec_preference_strings()
     |> validate_required(@required_fields)
   end
-
-  defp convert_codec_preference_strings(changeset) do
-    fields = [
-      video_codec_preference_string: :video_codec_preference,
-      audio_codec_preference_string: :audio_codec_preference
-    ]
-
-    Enum.reduce(fields, changeset, fn {virtual_field, actual_field}, changeset ->
-      case get_change(changeset, virtual_field) do
-        nil ->
-          changeset
-
-        value ->
-          new_value =
-            value
-            |> String.split(">")
-            |> Enum.map(&String.trim/1)
-            |> Enum.reject(&(String.trim(&1) == ""))
-            |> Enum.map(&String.downcase/1)
-
-          put_change(changeset, actual_field, new_value)
-      end
-    end)
-  end
 end

lib/pinchflat_web/controllers/settings/setting_html.ex

@@ -1,8 +1,6 @@
 defmodule PinchflatWeb.Settings.SettingHTML do
   use PinchflatWeb, :html
 
-  alias Pinchflat.Downloading.CodecParser
-
   embed_templates "setting_html/*"
 
   @doc """

lib/pinchflat_web/controllers/settings/setting_html/codec_settings_help.html.heex

@@ -1,17 +1,17 @@
 <aside>
-  <h2 class="text-xl font-bold mb-2">Codec Preferences</h2>
+  <h2 class="text-xl font-bold mb-2">Codec Preference</h2>
   <section class="ml-2 md:ml-4 mb-2 max-w-prose">
-    <section>
-      Available video codecs:
-      <ul class="list-disc ml-8">
-        <li :for={{codec, _} <- CodecParser.video_codec_map()}><%= codec %></li>
-      </ul>
-    </section>
-    <section class="mt-4">
-      Available audio codecs:
-      <ul class="list-disc ml-8">
-        <li :for={{codec, _} <- CodecParser.audio_codec_map()}><%= codec %></li>
-      </ul>
-    </section>
+    <p>
+      Some users may want to specify the audio codec that <code class="text-sm">yt-dlp</code>
+      searches for when downloading content. For these users, you can specify one audio and video
+      codec to prefer above all others.
+    </p>
+    <p class="mt-4">
+      NOTE: this is a <em>soft</em>
+      preference. If the codec you specify is not available, <code class="text-sm">yt-dlp</code>
+      will download the next best available codec. This is expected and intended behavior.
+      <.inline_link href="https://github.com/yt-dlp/yt-dlp#sorting-formats">See here</.inline_link>
+      for more information and a list of available codecs.
+    </p>
   </section>
 </aside>

lib/pinchflat_web/controllers/settings/setting_html/setting_form.html.heex

@@ -37,24 +37,20 @@
       </p>
 
       <.input
-        id="video_codec_preference_string"
-        name="setting[video_codec_preference_string]"
-        value={Enum.join(f[:video_codec_preference].value, ">")}
-        placeholder="avc>vp9>av01"
+        field={f[:video_codec_preference]}
+        placeholder="avc"
         type="text"
         label="Video Codec Preference"
-        help="Order of preference for video codecs. Separate with >. Will be remuxed into an MP4 container. See below for available codecs"
+        help="Video codec preference. Will be remuxed into an MP4 container. See below for more details"
         inputclass="font-mono text-sm mr-4"
       />
 
       <.input
-        id="audio_codec_preference_string"
-        name="setting[audio_codec_preference_string]"
-        value={Enum.join(f[:audio_codec_preference].value, ">")}
-        placeholder="mp4a>opus>aac"
+        field={f[:audio_codec_preference]}
+        placeholder="m4a"
         type="text"
         label="Audio Codec Preference"
-        help="Order of preference for audio codecs. Separate with >. See below for available codecs"
+        help="Audio codec preference. See below for more details"
         inputclass="font-mono text-sm mr-4"
       />
     </section>

priv/repo/migrations/20240603230512_refactor_codec_preferences.exs

@@ -0,0 +1,15 @@
+defmodule Pinchflat.Repo.Migrations.RefactorCodecPreferences do
+  use Ecto.Migration
+
+  def change do
+    alter table(:settings) do
+      remove :video_codec_preference, {:array, :string}, default: []
+      remove :audio_codec_preference, {:array, :string}, default: []
+    end
+
+    alter table(:settings) do
+      add :video_codec_preference, :string, default: "avc", null: false
+      add :audio_codec_preference, :string, default: "m4a", null: false
+    end
+  end
+end

test/pinchflat/downloading/codec_parser_test.exs

@@ -1,70 +0,0 @@
-defmodule Pinchflat.Downloading.CodecParserTest do
-  use Pinchflat.DataCase
-
-  alias Pinchflat.Settings
-  alias Pinchflat.Downloading.CodecParser
-
-  describe "generate_vcodec_string_from_settings/1" do
-    test "returns a default vcodec string when setting isn't set" do
-      Settings.set(video_codec_preference: [])
-
-      assert "bestvideo[vcodec~='^avc']/bestvideo" == CodecParser.generate_vcodec_string_from_settings()
-    end
-
-    test "generates a vcodec string" do
-      Settings.set(video_codec_preference: ["av01"])
-
-      assert "bestvideo[vcodec~='^av01']/bestvideo" == CodecParser.generate_vcodec_string_from_settings()
-    end
-  end
-
-  describe "generate_acodec_string_from_settings/1" do
-    test "returns a default acodec string when setting isn't set" do
-      Settings.set(audio_codec_preference: [])
-
-      assert "bestaudio[acodec~='^mp4a']/bestaudio" == CodecParser.generate_acodec_string_from_settings()
-    end
-
-    test "generates an acodec string" do
-      Settings.set(audio_codec_preference: ["mp3"])
-
-      assert "bestaudio[acodec~='^mp3']/bestaudio" == CodecParser.generate_acodec_string_from_settings()
-    end
-  end
-
-  describe "generate_vcodec_string/1" do
-    test "returns a default vcodec string when nil" do
-      assert "bestvideo[vcodec~='^avc']/bestvideo" == CodecParser.generate_vcodec_string(nil)
-    end
-
-    test "returns a default vcodec string when empty" do
-      assert "bestvideo[vcodec~='^avc']/bestvideo" == CodecParser.generate_vcodec_string([])
-    end
-
-    test "generates a vcodec string" do
-      assert "bestvideo[vcodec~='^av01']/bestvideo" == CodecParser.generate_vcodec_string(["av01"])
-    end
-
-    test "ignores options that don't exist" do
-      assert "bestvideo[vcodec~='^av01']/bestvideo" == CodecParser.generate_vcodec_string(["av01", "foo"])
-    end
-  end
-
-  describe "generate_acodec_string/1" do
-    test "returns a default acodec string when nil" do
-      assert "bestaudio[acodec~='^mp4a']/bestaudio" == CodecParser.generate_acodec_string(nil)
-    end
-
-    test "returns a default acodec string when empty" do
-      assert "bestaudio[acodec~='^mp4a']/bestaudio" == CodecParser.generate_acodec_string([])
-    end
-
-    test "generates an acodec string" do
-      assert "bestaudio[acodec~='^mp3']/bestaudio" == CodecParser.generate_acodec_string(["mp3"])
-    end
-
-    test "ignores options that don't exist" do
-      assert "bestaudio[acodec~='^mp3']/bestaudio" == CodecParser.generate_acodec_string(["mp3", "foo"])
-    end
-  end
-end

test/pinchflat/downloading/download_option_builder_test.exs

@@ -256,10 +256,8 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
         media_item = Repo.preload(media_item_fixture(source_id: source.id), source: :media_profile)
 
         assert {:ok, res} = DownloadOptionBuilder.build(media_item)
-        assert {:format_sort, "res:#{resolution}"} in res
-
-        assert {:format, "((bestvideo[vcodec~='^avc']/bestvideo)+(bestaudio[acodec~='^mp4a']/bestaudio))/best"} in res
 
+        assert {:format_sort, "res:#{resolution},+codec:avc:m4a"} in res
         assert {:remux_video, "mp4"} in res
       end)
     end
@@ -270,20 +268,20 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
       assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :extract_audio in res
-      assert {:format, "bestaudio[acodec~='^mp4a']/bestaudio/best"} in res
+      assert {:format_sort, "+acodec:m4a"} in res
 
       refute {:remux_video, "mp4"} in res
     end
 
     test "includes custom quality options if specified", %{media_item: media_item} do
-      Settings.set(video_codec_preference: ["av01"])
-      Settings.set(audio_codec_preference: ["aac"])
+      Settings.set(video_codec_preference: "av01")
+      Settings.set(audio_codec_preference: "aac")
 
       media_item = update_media_profile_attribute(media_item, %{preferred_resolution: :"1080p"})
 
       assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
-      assert {:format, "((bestvideo[vcodec~='^av01']/bestvideo)+(bestaudio[acodec~='^aac']/bestaudio))/best"} in res
+      assert {:format_sort, "res:1080,+codec:av01:aac"} in res
     end
   end
 

test/pinchflat/settings_test.exs

@@ -78,64 +78,4 @@ defmodule Pinchflat.SettingsTest do
       assert %Ecto.Changeset{} = Settings.change_setting(setting, %{onboarding: true})
     end
   end
-
-  describe "change_setting/2 when testing codec preferences" do
-    test "converts (video|audio)_codec_preference_string to an array" do
-      setting = Settings.record()
-
-      new_setting = %{
-        video_codec_preference_string: "avc>vp9",
-        audio_codec_preference_string: "aac>opus"
-      }
-
-      changeset = Settings.change_setting(setting, new_setting)
-
-      assert ["avc", "vp9"] = changeset.changes.video_codec_preference
-      assert ["aac", "opus"] = changeset.changes.audio_codec_preference
-    end
-
-    test "removes whitespace from (video|audio)_codec_preference" do
-      setting = Settings.record()
-
-      new_setting = %{
-        video_codec_preference_string: "  avc  > >  vp9  ",
-        audio_codec_preference_string: "aac> opus "
-      }
-
-      changeset = Settings.change_setting(setting, new_setting)
-
-      assert ["avc", "vp9"] = changeset.changes.video_codec_preference
-      assert ["aac", "opus"] = changeset.changes.audio_codec_preference
-    end
-
-    test "downcases (video|audio)_codec_preference" do
-      setting = Settings.record()
-
-      new_setting = %{
-        video_codec_preference_string: "AVC>VP9",
-        audio_codec_preference_string: "AAC>OPUS"
-      }
-
-      changeset = Settings.change_setting(setting, new_setting)
-
-      assert ["avc", "vp9"] = changeset.changes.video_codec_preference
-      assert ["aac", "opus"] = changeset.changes.audio_codec_preference
-    end
-
-    test "an empty value will remove the codec settings" do
-      Settings.set(video_codec_preference: ["avc", "vp9"])
-      Settings.set(audio_codec_preference: ["aac", "opus"])
-      setting = Settings.record()
-
-      new_setting = %{
-        video_codec_preference_string: "",
-        audio_codec_preference_string: ""
-      }
-
-      changeset = Settings.change_setting(setting, new_setting)
-
-      assert [] = changeset.changes.video_codec_preference
-      assert [] = changeset.changes.audio_codec_preference
-    end
-  end
 end