Repository: wandb/wandb
Pull request: chore: update video documentation
Description
-----------
<!--
Include reference to internal ticket "Fixes WB-NNNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
-->
- Fixes #3532
- Fixes WB-10374

What does the PR do? Include a concise description of the PR contents.

Updates the `wandb.video` documentation, to have a class and initialization documentation. As well as cleaning up the wording on the documentation when which parameters are necessary.

This PR also emits a warning when initializing a `wandb.Video` object without providing the format. In later versions we should have users explicitly define the video format, so the trade offs between formats is intentional between users. (trade offs such as encode times, file size, etc.) ref: #3532

<!--
NEW: We're using a new changelog format that's more useful for users. Please
see CHANGELOG.unreleased.md for details and update on relevant changes such as feature
additions, bug fixes, or removals/deprecations.
-->
- [x] I updated CHANGELOG.unreleased.md, or it's not applicable


Testing
-------
How was this PR tested?

<!--
Ensure PR title compliance with the [conventional commits standards](https://github.com/wandb/wandb/blob/main/CONTRIBUTING.md#conventional-commits)
-->


Problem statement:
[CLI]: wandb.Video takes a very long time
### Describe the bug

Running wandb.Video when providing a numpy array takes a very long time. Using the profiler I found that the issue is the quantization of the frames for the GIF that is created by default. I suggest changing the format to be mp4 by default, this avoids this quantization and runs much faster. Also, it might be wise to change the default FPS to something other than 4. 

If the above changes are not desired for some reason, I suggest you update the documentation on the website to inform of this behavior and maybe also add an FAQ item for this.

Thanks!

### Additional Files

_No response_

### Environment

WandB version:
0.12.14

OS:
Fedora 8.5 server
Python version:
3.7.13
Versions of relevant libraries:
imageio=2.17.0
imageio-ffmpeg=0.4.7
moviepy=1.0.3
Pillow=9.0.1

### Additional Context

_No response_

Hints from issue discussion:
Hi @eliahuhorwitz,

Thank you for the request! I'll pass it along to our engineering team for review. Could you share a link to the page where you see this slowness?

Thanks,
Ramit
WandB Internal User commented: 
ramit-wandb commented: 
Hi @eliahuhorwitz,

Thank you for the request! I'll pass it along to our engineering team for review. Could you share a link to the page where you see this slowness?

Thanks,
Ramit

> Hi @eliahuhorwitz,
> 
> Thank you for the request! I'll pass it along to our engineering team for review. Could you share a link to the page where you see this slowness?
> 
> Thanks, Ramit

Hey @ramit-wandb,
Thanks for the prompt answer. I am seeing the slowness in the python library, not the website. As for the documentation, I followed the code snippet found here https://docs.wandb.ai/ref/python/log#video-from-numpy and the documentation found here https://docs.wandb.ai/guides/track/log/media#other-media, here https://docs.wandb.ai/guides/track/log/media#how-do-i-log-a-video and here https://docs.wandb.ai/ref/python/data-types/video. Nowhere does it hint to the impact of gifs vs. mp4s on performance. 

Cheers,
Eliahu

Leslie commented: 
Thank you for the information Eliahu! We'll create a ticket for this and let you know if there are any updates on it

Commit message: chore: update video documentation

Patch to review:
diff --git a/BREAKING.md b/BREAKING.md
index f0f54be8e72..9beb2e100e6 100644
--- a/BREAKING.md
+++ b/BREAKING.md
@@ -69,3 +69,7 @@ When preparing a release that can include breaking changes, consider applying ch
 - Remove fallback of storing system settings in a temporary directory when we don't have permissions to write to `~/.config/wandb/settings`
     - Owner: @jacobromero
     - Can do in >=0.20
+
+- Require `format` argument when initializing `wandb.Video`
+    - Owner: @jacobromero
+    - can do in >=0.20
diff --git a/wandb/sdk/data_types/video.py b/wandb/sdk/data_types/video.py
index f765357ce01..20172b5d092 100644
--- a/wandb/sdk/data_types/video.py
+++ b/wandb/sdk/data_types/video.py
@@ -2,7 +2,7 @@
 import logging
 import os
 from io import BytesIO
-from typing import TYPE_CHECKING, Any, Optional, Sequence, Type, Union
+from typing import TYPE_CHECKING, Any, Literal, Optional, Sequence, Type, Union
 
 import wandb
 from wandb import util
@@ -48,36 +48,7 @@ def write_gif_with_image_io(
 
 
 class Video(BatchableMedia):
-    """Format a video for logging to W&B.
-
-    Args:
-        data_or_path: (numpy array, string, io)
-            Video can be initialized with a path to a file or an io object.
-            The format must be "gif", "mp4", "webm" or "ogg".
-            The format must be specified with the format argument.
-            Video can be initialized with a numpy tensor.
-            The numpy tensor must be either 4 dimensional or 5 dimensional.
-            Channels should be (time, channel, height, width) or
-            (batch, time, channel, height width)
-        caption: (string) caption associated with the video for display
-        fps: (int)
-            The frame rate to use when encoding raw video frames. Default value is 4.
-            This parameter has no effect when data_or_path is a string, or bytes.
-        format: (string) format of video, necessary if initializing with path or io object.
-
-    Examples:
-        ### Log a numpy array as a video
-        <!--yeadoc-test:log-video-numpy-->
-        ```python
-        import numpy as np
-        import wandb
-
-        run = wandb.init()
-        # axes are (time, channel, height, width)
-        frames = np.random.randint(low=0, high=256, size=(10, 3, 100, 100), dtype=np.uint8)
-        run.log({"video": wandb.Video(frames, fps=4)})
-        ```
-    """
+    """A class for logging videos to W&B."""
 
     _log_type = "video-file"
     EXTS = ("gif", "mp4", "webm", "ogg")
@@ -89,10 +60,53 @@ def __init__(
         data_or_path: Union["np.ndarray", str, "TextIO", "BytesIO"],
         caption: Optional[str] = None,
         fps: Optional[int] = None,
-        format: Optional[str] = None,
+        format: Optional[Literal["gif", "mp4", "webm", "ogg"]] = None,
     ):
+        """Initialize a W&B Video object.
+
+        Args:
+            data_or_path:
+                Video can be initialized with a path to a file or an io object.
+                Video can be initialized with a numpy tensor.
+                The numpy tensor must be either 4 dimensional or 5 dimensional.
+                Channels should be (number of frames, channel, height, width) or
+                (batch, number of frames, channel, height, width)
+                Supported formats are "gif", "mp4", "webm" or "ogg".
+                The format must be specified with the format argument
+                when initializing with a numpy array
+                or io object.
+            caption: Caption associated with the video for display.
+            fps:
+                The frame rate to use when encoding raw video frames.
+                Default value is 4.
+                This parameter has no effect when data_or_path is a string, or bytes.
+            format:
+                Format of video, necessary if initializing with a numpy array or io object.
+                This parameter will be used to determine the format to use when encoding the video data.
+                Accepted values are "gif", "mp4", "webm", or "ogg".
+
+        Examples:
+            ### Log a numpy array as a video
+            ```python
+            import numpy as np
+            import wandb
+
+            run = wandb.init()
+            # axes are (number of frames, channel, height, width)
+            frames = np.random.randint(
+                low=0, high=256, size=(10, 3, 100, 100), dtype=np.uint8
+            )
+            run.log({"video": wandb.Video(frames, format="mp4", fps=4)})
+            ```
+        """
         super().__init__(caption=caption)
 
+        if format is None:
+            wandb.termwarn(
+                "`format` argument was not provided, defaulting to `gif`. "
+                "This parameter will be required in v0.20.0, "
+                "please specify the format explicitly."
+            )
         self._format = format or "gif"
         self._width = None
         self._height = None


Code review comments:
diff_hunk: @@ -89,10 +60,53 @@ def __init__(
         data_or_path: Union["np.ndarray", str, "TextIO", "BytesIO"],
         caption: Optional[str] = None,
         fps: Optional[int] = None,
-        format: Optional[str] = None,
+        format: Optional[Literal["gif", "mp4", "webm", "ogg"]] = None,
     ):
+        """Initialize a W&B Video object.
+
+        Args:
+            data_or_path:
+                Video can be initialized with a path to a file or an io object.
+                Video can be initialized with a numpy tensor.
+                The numpy tensor must be either 4 dimensional or 5 dimensional.
+                Channels should be (number of frames, channel, height, width) or
+                (batch, number of frames, channel, height, width)
+                Supported formats are "gif", "mp4", "webm" or "ogg".
+                The format must be specified with the format argument
+                when initializing with a numpy array
+                or io object.
+            caption: Caption associated with the video for display.
+            fps:
+                The frame rate to use when encoding raw video frames.
+                Default value is 4.
+                This parameter has no effect when data_or_path is a string, or bytes.
+            format:
+                Format of video, necessary if initializing with a numpy array or io object.
+                This parameter will be used to determine the format to use when encoding the video data.
+                Accepted values are "gif", "mp4", "webm", or "ogg".
+
+        Examples:
+            ### Log a numpy array as a video
+            ```python
+            import numpy as np
+            import wandb
+
+            run = wandb.init()
+            # axes are (number of frames, channel, height, width)
+            frames = np.random.randint(
+                low=0, high=256, size=(10, 3, 100, 100), dtype=np.uint8
+            )
+            run.log({"video": wandb.Video(frames, format="mp4", fps=4)})
path: wandb/sdk/data_types/video.py
text: @user1:
no finish?

@author:
ah you are right, meant to update this section, better to push for a context manager as an example here. Let me update this!