Fix MKV attachment cleanup detection tests

Author: mjc

lib/reencodarr/ab_av1/helper.ex

@@ -107,16 +107,33 @@ defmodule Reencodarr.AbAv1.Helper do
 
   @spec parse_attached_pictures(String.t()) :: {:ok, list(map())}
   defp parse_attached_pictures(output) do
-    case Jason.decode(output) do
-      {:ok, %{"streams" => streams}} ->
-        pics = Enum.filter(streams, &attached_picture?/1)
-        {:ok, pics}
+    case extract_ffprobe_json(output) do
+      {:ok, json} ->
+        case Jason.decode(json) do
+          {:ok, %{"streams" => streams}} ->
+            pics = Enum.filter(streams, &attached_picture?/1)
+            {:ok, pics}
+
+          _ ->
+            {:ok, []}
+        end
 
-      _ ->
+      :error ->
         {:ok, []}
     end
   end
 
+  defp extract_ffprobe_json(output) do
+    case :binary.match(output, "{") do
+      {idx, _len} ->
+        json = binary_part(output, idx, byte_size(output) - idx) |> String.trim()
+        {:ok, json}
+
+      :nomatch ->
+        :error
+    end
+  end
+
   @spec attached_picture?(map()) :: boolean()
   defp attached_picture?(stream) do
     get_in(stream, ["disposition", "attached_pic"]) == 1 or
@@ -135,23 +152,42 @@ defmodule Reencodarr.AbAv1.Helper do
 
   @spec clean_mkv_in_place(String.t(), list(map())) :: {:ok, String.t()}
   defp clean_mkv_in_place(file_path, attached_pics) do
-    # Step 1: Delete image attachments via mkvpropedit (in-place, fast)
-    delete_image_attachments(file_path)
-
-    # Step 2: Check if any are actual MJPEG video tracks (not just attachments)
     has_mjpeg_tracks =
       Enum.any?(attached_pics, fn s ->
         s["codec_type"] == "video" and s["codec_name"] == "mjpeg" and
           get_in(s, ["disposition", "attached_pic"]) != 1
       end)
 
-    if has_mjpeg_tracks do
-      # mkvpropedit can't delete tracks — fall back to ffmpeg remux
-      Logger.info("Found MJPEG video tracks in #{file_path}, using ffmpeg remux")
-      clean_via_ffmpeg_remux(file_path)
-    else
-      Logger.info("Successfully cleaned image attachments from #{file_path} (in-place)")
-      {:ok, file_path}
+    # Step 1: Delete image attachments via mkvpropedit (in-place, fast)
+    delete_image_attachments(file_path)
+
+    case detect_attached_pictures(file_path) do
+      {:ok, []} when has_mjpeg_tracks ->
+        # mkvpropedit only handles attachments, not real tracks.
+        Logger.info("Found MJPEG video tracks in #{file_path}, using ffmpeg remux")
+        clean_via_ffmpeg_remux(file_path)
+
+      {:ok, []} ->
+        Logger.info("Successfully cleaned image attachments from #{file_path} (in-place)")
+        {:ok, file_path}
+
+      {:ok, remaining} ->
+        Logger.warning(
+          "mkvpropedit left #{length(remaining)} image stream(s) in #{file_path}; falling back to ffmpeg remux"
+        )
+
+        clean_via_ffmpeg_remux(file_path)
+
+      {:error, reason} ->
+        Logger.warning(
+          "Failed to verify MKV attachment cleanup for #{file_path}: #{inspect(reason)}"
+        )
+
+        if has_mjpeg_tracks do
+          clean_via_ffmpeg_remux(file_path)
+        else
+          {:ok, file_path}
+        end
     end
   end
 

test/reencodarr/ab_av1/helper_test.exs

@@ -69,7 +69,7 @@ defmodule Reencodarr.AbAv1.HelperTest do
     test "detects streams with attached_pic disposition" do
       file_path = "/media/movie_with_cover.mkv"
 
-      ffprobe_response =
+      ffprobe_with_images =
         Jason.encode!(%{
           "streams" => [
             %{
@@ -86,11 +86,30 @@ defmodule Reencodarr.AbAv1.HelperTest do
           ]
         })
 
+      ffprobe_clean =
+        Jason.encode!(%{
+          "streams" => [
+            %{
+              "codec_name" => "hevc",
+              "codec_type" => "video",
+              "disposition" => %{"attached_pic" => 0}
+            }
+          ]
+        })
+
+      counter = :counters.new(1, [:atomics])
       :meck.new(System, [:passthrough])
 
       :meck.expect(System, :cmd, fn
-        "ffprobe", _, _ -> {ffprobe_response, 0}
-        "mkvpropedit", _, _ -> {"Done.\n", 0}
+        "ffprobe", _, _ ->
+          :counters.add(counter, 1, 1)
+
+          if :counters.get(counter, 1) == 1,
+            do: {ffprobe_with_images, 0},
+            else: {ffprobe_clean, 0}
+
+        "mkvpropedit", _, _ ->
+          {"Done.\n", 0}
       end)
 
       assert {:ok, ^file_path} = Helper.clean_attachments(file_path)
@@ -135,7 +154,7 @@ defmodule Reencodarr.AbAv1.HelperTest do
     test "calls mkvpropedit in-place for MKV with image attachments" do
       file_path = "/media/movie.mkv"
 
-      ffprobe_response =
+      ffprobe_with_images =
         Jason.encode!(%{
           "streams" => [
             %{
@@ -152,11 +171,30 @@ defmodule Reencodarr.AbAv1.HelperTest do
           ]
         })
 
+      ffprobe_clean =
+        Jason.encode!(%{
+          "streams" => [
+            %{
+              "codec_name" => "hevc",
+              "codec_type" => "video",
+              "disposition" => %{"attached_pic" => 0}
+            }
+          ]
+        })
+
+      counter = :counters.new(1, [:atomics])
       :meck.new(System, [:passthrough])
 
       :meck.expect(System, :cmd, fn
-        "ffprobe", _, _ -> {ffprobe_response, 0}
-        "mkvpropedit", [^file_path | _], _ -> {"Done.\n", 0}
+        "ffprobe", _, _ ->
+          :counters.add(counter, 1, 1)
+
+          if :counters.get(counter, 1) == 1,
+            do: {ffprobe_with_images, 0},
+            else: {ffprobe_clean, 0}
+
+        "mkvpropedit", [^file_path | _], _ ->
+          {"Done.\n", 0}
       end)
 
       assert {:ok, ^file_path} = Helper.clean_attachments(file_path)
@@ -171,7 +209,7 @@ defmodule Reencodarr.AbAv1.HelperTest do
     test "does not create temp files for MKV attachment-only case" do
       file_path = "/media/movie.MKV"
 
-      ffprobe_response =
+      ffprobe_with_images =
         Jason.encode!(%{
           "streams" => [
             %{"codec_name" => "hevc", "codec_type" => "video"},
@@ -184,11 +222,30 @@ defmodule Reencodarr.AbAv1.HelperTest do
           ]
         })
 
+      ffprobe_clean =
+        Jason.encode!(%{
+          "streams" => [
+            %{
+              "codec_name" => "hevc",
+              "codec_type" => "video",
+              "disposition" => %{"attached_pic" => 0}
+            }
+          ]
+        })
+
+      counter = :counters.new(1, [:atomics])
       :meck.new(System, [:passthrough])
 
       :meck.expect(System, :cmd, fn
-        "ffprobe", _, _ -> {ffprobe_response, 0}
-        "mkvpropedit", _, _ -> {"Done.\n", 0}
+        "ffprobe", _, _ ->
+          :counters.add(counter, 1, 1)
+
+          if :counters.get(counter, 1) == 1,
+            do: {ffprobe_with_images, 0},
+            else: {ffprobe_clean, 0}
+
+        "mkvpropedit", _, _ ->
+          {"Done.\n", 0}
       end)
 
       {:ok, result_path} = Helper.clean_attachments(file_path)