fix: use video/mp4 as the mime type for mp4 files (#8377)

* refactor: move primary_mime_type() to tr_torrent_files where it is easier to test

* fix: use video/mp4 mime type for .mp4 files

* docs: remove obsolete code comment

libtransmission/torrent-files.cc

@@ -18,6 +18,8 @@
 
 #include <fmt/format.h>
 
+#include <small/map.hpp>
+
 #include "libtransmission/transmission.h"
 
 #include "libtransmission/error.h"
@@ -156,6 +158,31 @@ bool tr_torrent_files::has_any_local_data(std::string_view const* paths, size_t
     return false;
 }
 
+std::string_view tr_torrent_files::primary_mime_type() const
+{
+    // count up how many bytes there are for each mime-type in the torrent
+    auto size_per_mime_type = small::unordered_map<std::string_view, size_t, 256U>{};
+    for (tr_file_index_t i = 0, n = file_count(); i < n; ++i)
+    {
+        auto const mime_type = tr_get_mime_type_for_filename(path(i));
+        size_per_mime_type[mime_type] += file_size(i);
+    }
+
+    if (std::empty(size_per_mime_type))
+    {
+        // https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
+        // application/octet-stream is the default value for all other cases.
+        // An unknown file type should use this type.
+        auto constexpr Fallback = "application/octet-stream"sv;
+        return Fallback;
+    }
+
+    auto const it = std::ranges::max_element(
+        size_per_mime_type,
+        [](auto const& a, auto const& b) { return a.second < b.second; });
+    return it->first;
+}
+
 // ---
 
 bool tr_torrent_files::move(

libtransmission/torrent-files.h

@@ -157,6 +157,7 @@ public:
 
     [[nodiscard]] std::optional<FoundFile> find(tr_file_index_t file, std::string_view const* paths, size_t n_paths) const;
     [[nodiscard]] bool has_any_local_data(std::string_view const* paths, size_t n_paths) const;
+    [[nodiscard]] std::string_view primary_mime_type() const;
 
     static void sanitize_subpath(std::string_view path, tr_pathbuf& append_me, bool os_specific = true);
 

libtransmission/torrent.cc

@@ -20,8 +20,6 @@
 #include <fmt/chrono.h>
 #include <fmt/format.h>
 
-#include <small/map.hpp>
-
 #include "libtransmission/transmission.h"
 #include "libtransmission/tr-macros.h"
 
@@ -2097,29 +2095,7 @@ uint64_t tr_torrentGetBytesLeftToAllocate(tr_torrent const* tor)
 
 std::string_view tr_torrent::primary_mime_type() const
 {
-    // count up how many bytes there are for each mime-type in the torrent
+    return files().primary_mime_type();
-    // NB: get_mime_type_for_filename() always returns the same ptr for a
-    // mime_type, so its raw pointer can be used as a key.
-    auto size_per_mime_type = small::unordered_map<std::string_view, size_t, 256U>{};
-    for (tr_file_index_t i = 0, n = this->file_count(); i < n; ++i)
-    {
-        auto const mime_type = tr_get_mime_type_for_filename(this->file_subpath(i));
-        size_per_mime_type[mime_type] += this->file_size(i);
-    }
-
-    if (std::empty(size_per_mime_type))
-    {
-        // https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
-        // application/octet-stream is the default value for all other cases.
-        // An unknown file type should use this type.
-        auto constexpr Fallback = "application/octet-stream"sv;
-        return Fallback;
-    }
-
-    auto const it = std::ranges::max_element(
-        size_per_mime_type,
-        [](auto const& a, auto const& b) { return a.second < b.second; });
-    return it->first;
 }
 
 // ---

libtransmission/utils.cc

@@ -797,7 +797,18 @@ std::string_view tr_get_mime_type_for_filename(std::string_view filename)
         auto const it = std::lower_bound(std::begin(MimeTypeSuffixes), std::end(MimeTypeSuffixes), suffix_lc, Compare);
         if (it != std::end(MimeTypeSuffixes) && suffix_lc == it->suffix)
         {
-            return it->mime_type;
+            std::string_view mime_type = it->mime_type;
+
+            // https://github.com/transmission/transmission/issues/5965#issuecomment-1704421231
+            // An mp4 file's correct mime-type depends on the codecs used in the file,
+            // which we have no way of inspecting and which might not be downloaded yet.
+            // Let's use `video/mp4` since that's by far the most common use case for torrents.
+            if (mime_type == "application/mp4")
+            {
+                mime_type = "video/mp4";
+            }
+
+            return mime_type;
         }
     }
 

tests/libtransmission/torrent-files-test.cc

@@ -17,6 +17,7 @@
 
 #include <libtransmission/file.h>
 #include <libtransmission/torrent-files.h>
+#include <libtransmission/torrent-metainfo.h>
 #include "libtransmission/tr-macros.h"
 #include <libtransmission/tr-strbuf.h>
 
@@ -143,6 +144,23 @@ TEST_F(TorrentFilesTest, hasAnyLocalData)
     EXPECT_FALSE(files.has_any_local_data(std::data(search_path), 0U));
 }
 
+TEST_F(TorrentFilesTest, mimeType)
+{
+    auto const filename = tr_pathbuf{ LIBTRANSMISSION_TEST_ASSETS_DIR, "/alice_in_wonderland_librivox_archive.torrent"sv };
+    auto metainfo = tr_torrent_metainfo{};
+    EXPECT_TRUE(metainfo.parse_torrent_file(filename));
+    EXPECT_EQ("audio/mpeg"sv, metainfo.files().primary_mime_type());
+}
+
+TEST_F(TorrentFilesTest, mimeTypeVideoMp4)
+{
+    auto files = tr_torrent_files{};
+    files.add("name/name.mp4"sv, 4'500'000'000U);
+    files.add("name/name.info"sv, 2048U);
+    files.add("name/SHA512sum"sv, 139U);
+    EXPECT_EQ("video/mp4"sv, files.primary_mime_type());
+}
+
 TEST_F(TorrentFilesTest, isSubpathPortable)
 {
     static auto constexpr NotWin32 = TR_IF_WIN32(false, true);