Refactor MediaType guessing (rerun-io#7326)

### What * Closes rerun-io#7280 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7326?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7326?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7326) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`.
rickyli888 · Sep 2, 2024 · d11d468 · d11d468
1 parent 73bbbab
commit d11d468
Show file tree

Hide file tree

Showing 12 changed files with 93 additions and 128 deletions.
diff --git a/rerun_cpp/src/rerun/archetypes/asset3d.hpp b/rerun_cpp/src/rerun/archetypes/asset3d.hpp
diff --git a/rerun_cpp/src/rerun/archetypes/asset3d_ext.cpp b/rerun_cpp/src/rerun/archetypes/asset3d_ext.cpp
@@ -38,11 +38,6 @@ namespace rerun::archetypes {
             return asset;
         }
 
-
-        static std::optional<rerun::components::MediaType> guess_media_type(
-            const std::filesystem::path& path
-        );
-
         // </CODEGEN_COPY_TO_HEADER>
 #endif
 
@@ -61,27 +56,7 @@ namespace rerun::archetypes {
 
         return Asset3D::from_bytes(
             Collection<uint8_t>::take_ownership(std::move(data)),
-            Asset3D::guess_media_type(path)
+            rerun::components::MediaType::guess_from_path(path)
         );
     }
-
-    std::optional<rerun::components::MediaType> Asset3D::guess_media_type(
-        const std::filesystem::path& path
-    ) {
-        std::filesystem::path file_path(path);
-        std::string ext = file_path.extension().string();
-        std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower);
-
-        if (ext == ".glb") {
-            return rerun::components::MediaType::glb();
-        } else if (ext == ".gltf") {
-            return rerun::components::MediaType::gltf();
-        } else if (ext == ".obj") {
-            return rerun::components::MediaType::obj();
-        } else if (ext == ".stl") {
-            return rerun::components::MediaType::stl();
-        } else {
-            return std::nullopt;
-        }
-    }
 } // namespace rerun::archetypes
diff --git a/rerun_cpp/src/rerun/archetypes/asset_video.hpp b/rerun_cpp/src/rerun/archetypes/asset_video.hpp
diff --git a/rerun_cpp/src/rerun/archetypes/asset_video_ext.cpp b/rerun_cpp/src/rerun/archetypes/asset_video_ext.cpp
@@ -38,11 +38,6 @@ namespace rerun::archetypes {
             return asset;
         }
 
-
-        static std::optional<rerun::components::MediaType> guess_media_type(
-            const std::filesystem::path& path
-        );
-
         // </CODEGEN_COPY_TO_HEADER>
 #endif
 
@@ -61,21 +56,7 @@ namespace rerun::archetypes {
 
         return AssetVideo::from_bytes(
             Collection<uint8_t>::take_ownership(std::move(data)),
-            AssetVideo::guess_media_type(path)
+            rerun::components::MediaType::guess_from_path(path)
         );
     }
-
-    std::optional<rerun::components::MediaType> AssetVideo::guess_media_type(
-        const std::filesystem::path& path
-    ) {
-        std::filesystem::path file_path(path);
-        std::string ext = file_path.extension().string();
-        std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower);
-
-        if (ext == ".mp4") {
-            return rerun::components::MediaType::mp4();
-        } else {
-            return std::nullopt;
-        }
-    }
 } // namespace rerun::archetypes
diff --git a/rerun_cpp/src/rerun/archetypes/encoded_image.hpp b/rerun_cpp/src/rerun/archetypes/encoded_image.hpp
diff --git a/rerun_cpp/src/rerun/archetypes/encoded_image_ext.cpp b/rerun_cpp/src/rerun/archetypes/encoded_image_ext.cpp
@@ -40,10 +40,6 @@ namespace rerun::archetypes {
         return image;
     }
 
-    static std::optional<rerun::components::MediaType> guess_media_type(
-        const std::filesystem::path& path
-    );
-
     // </CODEGEN_COPY_TO_HEADER>
 #endif
 
@@ -64,22 +60,9 @@ namespace rerun::archetypes {
             return Error(ErrorCode::FileRead, filepath.string());
         }
 
-        return EncodedImage::from_bytes(file_bytes, EncodedImage::guess_media_type(filepath));
-    }
-
-    std::optional<rerun::components::MediaType> EncodedImage::guess_media_type(
-        const std::filesystem::path& path
-    ) {
-        std::filesystem::path file_path(path);
-        std::string ext = file_path.extension().string();
-        std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower);
-
-        if (ext == ".jpg" || ext == ".jpeg") {
-            return rerun::components::MediaType::jpeg();
-        } else if (ext == ".png") {
-            return rerun::components::MediaType::png();
-        } else {
-            return std::nullopt;
-        }
+        return EncodedImage::from_bytes(
+            file_bytes,
+            rerun::components::MediaType::guess_from_path(filepath)
+        );
     }
 } // namespace rerun::archetypes
diff --git a/rerun_cpp/src/rerun/components/media_type.hpp b/rerun_cpp/src/rerun/components/media_type.hpp
diff --git a/rerun_cpp/src/rerun/components/media_type_ext.cpp b/rerun_cpp/src/rerun/components/media_type_ext.cpp
@@ -1,8 +1,19 @@
+#include <algorithm>
+#include <optional>
+#include <string>
 #include "media_type.hpp"
 
 // Uncomment for better auto-complete while editing the extension.
 // #define EDIT_EXTENSION
 
+// It's undefined behavior to pre-declare std types, see http://www.gotw.ca/gotw/034.htm
+// We want to use `std::filesystem::path`, so we have it include it in the header.
+// <CODEGEN_COPY_TO_HEADER>
+
+#include <filesystem>
+
+// </CODEGEN_COPY_TO_HEADER>
+
 namespace rerun {
     namespace components {
 
@@ -86,9 +97,42 @@ namespace rerun {
                 return "video/mp4";
             }
 
+            static std::optional<MediaType> guess_from_path(const std::filesystem::path& path);
+
             // </CODEGEN_COPY_TO_HEADER>
         }
-    };
 #endif
-} // namespace components
-} // namespace rerun
+
+        std::optional<MediaType>
+            MediaType::guess_from_path(const std::filesystem::path& path) {
+            std::filesystem::path file_path(path);
+            std::string ext = file_path.extension().string();
+            std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower);
+
+            // Images
+            if (ext == ".jpg" || ext == ".jpeg") {
+                return rerun::components::MediaType::jpeg();
+            } else if (ext == ".png") {
+                return rerun::components::MediaType::png();
+            }
+
+            // 3D Models
+            if (ext == ".glb") {
+                return MediaType::glb();
+            } else if (ext == ".gltf") {
+                return MediaType::gltf();
+            } else if (ext == ".obj") {
+                return MediaType::obj();
+            } else if (ext == ".stl") {
+                return MediaType::stl();
+            }
+
+            // Video
+            if (ext == ".mp4") {
+                return MediaType::mp4();
+            }
+
+            return std::nullopt;
+        }
+    }; // namespace components
+};     // namespace rerun
diff --git a/rerun_py/rerun_sdk/rerun/archetypes/asset3d_ext.py b/rerun_py/rerun_sdk/rerun/archetypes/asset3d_ext.py
@@ -1,30 +1,11 @@
 from __future__ import annotations
 
 import pathlib
-from typing import TYPE_CHECKING, Any
+from typing import Any
 
 from .. import datatypes
 from ..error_utils import catch_and_log_exceptions
 
-if TYPE_CHECKING:
-    from ..components import MediaType
-
-
-def guess_media_type(path: str | pathlib.Path) -> MediaType | None:
-    from ..components import MediaType
-
-    ext = pathlib.Path(path).suffix.lower()
-    if ext == ".glb":
-        return MediaType.GLB
-    elif ext == ".gltf":
-        return MediaType.GLTF
-    elif ext == ".obj":
-        return MediaType.OBJ
-    elif ext == ".stl":
-        return MediaType.STL
-    else:
-        return None
-
 
 class Asset3DExt:
     """Extension for [Asset3D][rerun.archetypes.Asset3D]."""
@@ -64,6 +45,8 @@ def __init__(
 
         """
 
+        from ..components import MediaType
+
         with catch_and_log_exceptions(context=self.__class__.__name__):
             if (path is None) == (contents is None):
                 raise ValueError("Must provide exactly one of 'path' or 'contents'")
@@ -73,7 +56,7 @@ def __init__(
             else:
                 blob = pathlib.Path(path).read_bytes()
                 if media_type is None:
-                    media_type = guess_media_type(str(path))
+                    media_type = MediaType.guess_from_path(path)
 
             self.__attrs_init__(blob=blob, media_type=media_type)
             return

diff --git a/rerun_py/rerun_sdk/rerun/archetypes/asset_video_ext.py b/rerun_py/rerun_sdk/rerun/archetypes/asset_video_ext.py
@@ -10,16 +10,6 @@
     from ..components import MediaType
 
 
-def guess_media_type(path: str | pathlib.Path) -> MediaType | None:
-    from ..components import MediaType
-
-    ext = pathlib.Path(path).suffix.lower()
-    if ext == ".mp4":
-        return MediaType.MP4
-    else:
-        return None
-
-
 class AssetVideoExt:
     """Extension for [AssetVideo][rerun.archetypes.AssetVideo]."""
 
@@ -64,7 +54,7 @@ def __init__(
             else:
                 blob = pathlib.Path(path).read_bytes()
                 if media_type is None:
-                    media_type = guess_media_type(str(path))
+                    media_type = MediaType.guess_from_path(path)
 
             self.__attrs_init__(blob=blob, media_type=media_type)
             return

diff --git a/rerun_py/rerun_sdk/rerun/archetypes/encoded_image_ext.py b/rerun_py/rerun_sdk/rerun/archetypes/encoded_image_ext.py
@@ -10,7 +10,6 @@
 from ..error_utils import catch_and_log_exceptions
 
 if TYPE_CHECKING:
-    from ..components import MediaType
     from ..datatypes import Float32Like
 
     ImageLike = Union[
@@ -28,18 +27,6 @@
     ]
 
 
-def guess_media_type(path: str | pathlib.Path) -> MediaType | None:
-    from ..components import MediaType
-
-    ext = pathlib.Path(path).suffix.lower()
-    if ext == ".jpg" or ext == ".jpeg":
-        return MediaType.JPEG
-    elif ext == ".png":
-        return MediaType.PNG
-    else:
-        return None
-
-
 class EncodedImageExt:
     """Extension for [EncodedImage][rerun.archetypes.EncodedImage]."""
 
@@ -87,6 +74,8 @@ def __init__(
 
         """
 
+        from ..components import MediaType
+
         with catch_and_log_exceptions(context=self.__class__.__name__):
             if (path is None) == (contents is None):
                 raise ValueError("Must provide exactly one of 'path' or 'contents'")
@@ -100,7 +89,7 @@ def __init__(
                 blob = pathlib.Path(path).read_bytes()
 
                 if media_type is None:
-                    media_type = guess_media_type(str(path))
+                    media_type = MediaType.guess_from_path(path)
 
             self.__attrs_init__(blob=blob, media_type=media_type, draw_order=draw_order, opacity=opacity)
             return

diff --git a/rerun_py/rerun_sdk/rerun/components/media_type_ext.py b/rerun_py/rerun_sdk/rerun/components/media_type_ext.py
@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
 if TYPE_CHECKING:
@@ -91,3 +92,31 @@ def deferred_patch_class(cls: Any) -> None:
         cls.STL = cls("model/stl")
 
         cls.MP4 = cls("video/mp4")
+
+    @staticmethod
+    def guess_from_path(path: str | Path) -> MediaType | None:
+        from ..components import MediaType
+
+        ext = Path(path).suffix.lower()
+
+        # Images
+        if ext == ".jpg" or ext == ".jpeg":
+            return MediaType.JPEG
+        elif ext == ".png":
+            return MediaType.PNG
+
+        # 3D Models
+        if ext == ".glb":
+            return MediaType.GLB
+        elif ext == ".gltf":
+            return MediaType.GLTF
+        elif ext == ".obj":
+            return MediaType.OBJ
+        elif ext == ".stl":
+            return MediaType.STL
+
+        # Video
+        if ext == ".mp4":
+            return MediaType.MP4
+
+        return None