File component framerate (#145)

* Add framerate option for File component * Generate new openapi.yaml * Remove duplicated code * Fix credo issues * Add framerate as property in FileComponent * Update lib/jellyfish/room.ex Co-authored-by: Przemysław Rożnawski <[email protected]> * Update lib/jellyfish/component/file.ex Co-authored-by: Przemysław Rożnawski <[email protected]> * Fixes after reivew * Update openapi --------- Co-authored-by: Przemysław Rożnawski <[email protected]>
fishjam-dev · Feb 5, 2024 · b88fbd5 · b88fbd5
1 parent f671621
commit b88fbd5
Show file tree

Hide file tree

Showing 10 changed files with 190 additions and 55 deletions.
diff --git a/lib/jellyfish/component/file.ex b/lib/jellyfish/component/file.ex
@@ -12,7 +12,8 @@ defmodule Jellyfish.Component.File do
   alias JellyfishWeb.ApiSpec.Component.File.Options
 
   @type properties :: %{
-          file_path: Path.t()
+          file_path: Path.t(),
+          framerate: non_neg_integer() | nil
         }
 
   @files_location "file_component_sources"
@@ -24,7 +25,9 @@ defmodule Jellyfish.Component.File do
     with {:ok, valid_opts} <- OpenApiSpex.cast_value(options, Options.schema()),
          :ok <- validate_file_path(valid_opts.filePath),
          path = expand_file_path(valid_opts.filePath),
-         {:ok, track_config} <- get_track_config(path) do
+         {:ok, framerate} <- validate_framerate(valid_opts.framerate),
+         {:ok, track_config} <-
+           get_track_config(path, framerate) do
       endpoint_spec =
         %FileEndpoint{
           rtc_engine: engine,
@@ -35,11 +38,22 @@ defmodule Jellyfish.Component.File do
 
       properties = valid_opts |> Map.from_struct()
 
-      {:ok, %{endpoint: endpoint_spec, properties: properties}}
+      new_framerate =
+        if track_config.type == :video do
+          {framerate, 1} = track_config.opts[:framerate]
+          framerate
+        else
+          nil
+        end
+
+      {:ok, %{endpoint: endpoint_spec, properties: %{properties | framerate: new_framerate}}}
     else
       {:error, [%OpenApiSpex.Cast.Error{reason: :missing_field, name: name}]} ->
         {:error, {:missing_parameter, name}}
 
+      {:error, [%OpenApiSpex.Cast.Error{reason: :invalid_type, value: value, path: [:framerate]}]} ->
+        {:error, {:invalid_framerate, value}}
+
       {:error, _reason} = error ->
         error
     end
@@ -65,22 +79,22 @@ defmodule Jellyfish.Component.File do
     [media_files_path, @files_location, file_path] |> Path.join() |> Path.expand()
   end
 
-  defp get_track_config(file_path) do
-    file_path |> Path.extname() |> do_get_track_config()
+  defp get_track_config(file_path, framerate) do
+    file_path |> Path.extname() |> do_get_track_config(framerate)
   end
 
-  defp do_get_track_config(".h264") do
+  defp do_get_track_config(".h264", framerate) do
     {:ok,
      %FileEndpoint.TrackConfig{
        type: :video,
        encoding: :H264,
        clock_rate: 90_000,
        fmtp: %FMTP{pt: 96},
-       opts: [framerate: {30, 1}]
+       opts: [framerate: {framerate || 30, 1}]
      }}
   end
 
-  defp do_get_track_config(".ogg") do
+  defp do_get_track_config(".ogg", nil) do
     {:ok,
      %FileEndpoint.TrackConfig{
        type: :audio,
@@ -90,5 +104,13 @@ defmodule Jellyfish.Component.File do
      }}
   end
 
-  defp do_get_track_config(_extension), do: {:error, :unsupported_file_type}
+  defp do_get_track_config(".ogg", _not_nil) do
+    {:error, :bad_parameter_framerate_for_audio}
+  end
+
+  defp do_get_track_config(_extension, _framerate), do: {:error, :unsupported_file_type}
+
+  defp validate_framerate(nil), do: {:ok, nil}
+  defp validate_framerate(num) when is_number(num) and num > 0, do: {:ok, num}
+  defp validate_framerate(other), do: {:error, {:invalid_framerate, other}}
 end
diff --git a/lib/jellyfish/component/hls/request_handler.ex b/lib/jellyfish/component/hls/request_handler.ex
@@ -82,7 +82,7 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
       {:ok, partial}
     else
       {:error, :file_not_found} ->
-        case is_preload_hint(room_id, filename) do
+        case preload_hint?(room_id, filename) do
           {:ok, true} ->
             wait_for_partial_ready(room_id, filename)
 
@@ -102,7 +102,7 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
           {:ok, String.t()} | {:error, atom()}
   def handle_manifest_request(room_id, partial) do
     with {:ok, last_partial} <- EtsHelper.get_recent_partial(room_id) do
-      unless is_partial_ready(partial, last_partial) do
+      unless partial_ready?(partial, last_partial) do
         wait_for_manifest_ready(room_id, partial, :manifest)
       end
 
@@ -117,7 +117,7 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
           {:ok, String.t()} | {:error, atom()}
   def handle_delta_manifest_request(room_id, partial) do
     with {:ok, last_partial} <- EtsHelper.get_delta_recent_partial(room_id) do
-      unless is_partial_ready(partial, last_partial) do
+      unless partial_ready?(partial, last_partial) do
         wait_for_manifest_ready(room_id, partial, :delta_manifest)
       end
 
@@ -176,11 +176,11 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
   end
 
   @impl true
-  def handle_cast({:is_partial_ready, partial, from, manifest}, state) do
+  def handle_cast({:partial_ready?, partial, from, manifest}, state) do
     state =
       state
       |> Map.fetch!(manifest)
-      |> handle_is_partial_ready(partial, from)
+      |> handle_partial_ready?(partial, from)
       |> then(&Map.put(state, manifest, &1))
 
     {:noreply, state}
@@ -217,7 +217,7 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
   ###
 
   defp wait_for_manifest_ready(room_id, partial, manifest) do
-    GenServer.cast(registry_id(room_id), {:is_partial_ready, partial, self(), manifest})
+    GenServer.cast(registry_id(room_id), {:partial_ready?, partial, self(), manifest})
 
     receive do
       :manifest_ready ->
@@ -243,7 +243,7 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
     partials_ready =
       waiting_pids
       |> Map.keys()
-      |> Enum.filter(fn partial -> is_partial_ready(partial, last_partial) end)
+      |> Enum.filter(fn partial -> partial_ready?(partial, last_partial) end)
 
     partials_ready
     |> Enum.flat_map(fn partial -> Map.fetch!(waiting_pids, partial) end)
@@ -254,8 +254,8 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
     %{status | waiting_pids: waiting_pids, last_partial: last_partial}
   end
 
-  defp handle_is_partial_ready(status, partial, from) do
-    if is_partial_ready(partial, status.last_partial) do
+  defp handle_partial_ready?(status, partial, from) do
+    if partial_ready?(partial, status.last_partial) do
       send(from, :manifest_ready)
       status
     else
@@ -268,7 +268,7 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
     end
   end
 
-  defp is_preload_hint(room_id, filename) do
+  defp preload_hint?(room_id, filename) do
     partial_sn = get_partial_sn(filename)
 
     with {:ok, recent_partial_sn} <- EtsHelper.get_recent_partial(room_id) do
@@ -310,11 +310,11 @@ defmodule Jellyfish.Component.HLS.RequestHandler do
     Enum.each(waiting_pids, fn pid -> send(pid, :preload_hint_ready) end)
   end
 
-  defp is_partial_ready(_partial, nil) do
+  defp partial_ready?(_partial, nil) do
     false
   end
 
-  defp is_partial_ready({segment_sn, partial_sn}, {last_segment_sn, last_partial_sn}) do
+  defp partial_ready?({segment_sn, partial_sn}, {last_segment_sn, last_partial_sn}) do
     cond do
       last_segment_sn > segment_sn -> true
       last_segment_sn < segment_sn -> false

diff --git a/lib/jellyfish/config_reader.ex b/lib/jellyfish/config_reader.ex
@@ -195,7 +195,7 @@ defmodule Jellyfish.ConfigReader do
     [node_basename, hostname | []] = String.split(node_name_value, "@")
 
     node_name =
-      if is_ip_address(hostname) do
+      if ip_address?(hostname) do
         node_name
       else
         Logger.info(
@@ -259,7 +259,7 @@ defmodule Jellyfish.ConfigReader do
   defp parse_mode("sname"), do: :shortnames
   defp parse_mode(other), do: raise("Invalid JF_DIST_MODE. Expected sname or name, got: #{other}")
 
-  defp is_ip_address(hostname) do
+  defp ip_address?(hostname) do
     case :inet.parse_address(String.to_charlist(hostname)) do
       {:ok, _} -> true
       {:error, _} -> false

diff --git a/lib/jellyfish/room.ex b/lib/jellyfish/room.ex
@@ -254,7 +254,8 @@ defmodule Jellyfish.Room do
     component_options = Map.delete(options, "s3")
 
     with :ok <- check_component_allowed(component_type, state),
-         {:ok, component} <- Component.new(component_type, component_options) do
+         {:ok, component} <-
+           Component.new(component_type, component_options) do
       state = put_in(state, [:components, component.id], component)
 
       if component_type == HLS do
@@ -280,6 +281,18 @@ defmodule Jellyfish.Room do
         Logger.warning("Unable to add component: file does not exist")
         {:reply, {:error, :file_does_not_exist}, state}
 
+      {:error, :bad_parameter_framerate_for_audio} ->
+        Logger.warning("Attempted to set framerate for audio component which is not supported.")
+
+        {:reply, {:error, :bad_parameter_framerate_for_audio}, state}
+
+      {:error, {:invalid_framerate, passed_framerate}} ->
+        Logger.warning(
+          "Invalid framerate value: #{passed_framerate}.  It has to be a positivie integer."
+        )
+
+        {:reply, {:error, :invalid_framerate}, state}
+
       {:error, :invalid_file_path} ->
         Logger.warning("Unable to add component: invalid file path")
         {:reply, {:error, :invalid_file_path}, state}

diff --git a/lib/jellyfish/room_service.ex b/lib/jellyfish/room_service.ex
@@ -212,7 +212,7 @@ defmodule Jellyfish.RoomService do
   def handle_info({:DOWN, _ref, :process, pid, reason}, state) do
     {room_id, state} = pop_in(state, [:rooms, pid])
 
-    Logger.warning("Process #{room_id} is down with reason: #{reason}")
+    Logger.warning("Process #{room_id} is down with reason: #{inspect(reason)}")
 
     Phoenix.PubSub.broadcast(Jellyfish.PubSub, room_id, :room_crashed)
     Event.broadcast_server_notification({:room_crashed, room_id})

diff --git a/lib/jellyfish_web/api_spec/component/file.ex b/lib/jellyfish_web/api_spec/component/file.ex
@@ -18,9 +18,15 @@ defmodule JellyfishWeb.ApiSpec.Component.File do
           type: :string,
           description:
             "Relative path to track file. Must be either OPUS encapsulated in Ogg or raw h264"
+        },
+        framerate: %Schema{
+          type: :integer,
+          description: "Framerate of video in a file. It is only valid for video track",
+          example: 30,
+          nullable: true
         }
       },
-      required: [:filePath]
+      required: [:filePath, :framerate]
     })
   end
 
@@ -39,6 +45,12 @@ defmodule JellyfishWeb.ApiSpec.Component.File do
           type: :string,
           description: "Path to track file. Must be either OPUS encapsulated in Ogg or raw h264",
           example: "/root/video.h264"
+        },
+        framerate: %Schema{
+          type: :integer,
+          description: "Framerate of video in a file. It is only valid for video track",
+          nullable: true,
+          example: 30
         }
       },
       required: [:filePath]

diff --git a/lib/jellyfish_web/controllers/component_controller.ex b/lib/jellyfish_web/controllers/component_controller.ex
@@ -63,9 +63,11 @@ defmodule JellyfishWeb.ComponentController do
   def create(conn, %{"room_id" => room_id} = params) do
     with component_options <- Map.get(params, "options", %{}),
          {:ok, component_type_string} <- Map.fetch(params, "type"),
-         {:ok, component_type} <- Component.parse_type(component_type_string),
+         {:ok, component_type} <-
+           Component.parse_type(component_type_string),
          {:ok, _room_pid} <- RoomService.find_room(room_id),
-         {:ok, component} <- Room.add_component(room_id, component_type, component_options) do
+         {:ok, component} <-
+           Room.add_component(room_id, component_type, component_options) do
       conn
       |> put_resp_content_type("application/json")
       |> put_status(:created)
@@ -86,6 +88,13 @@ defmodule JellyfishWeb.ComponentController do
       {:error, :incompatible_codec} ->
         {:error, :bad_request, "Incompatible video codec enforced in room #{room_id}"}
 
+      {:error, :invalid_framerate} ->
+        {:error, :bad_request, "Invalid framerate passed"}
+
+      {:error, :bad_parameter_framerate_for_audio} ->
+        {:error, :bad_request,
+         "Attempted to set framerate for audio component which is not supported."}
+
       {:error, :invalid_file_path} ->
         {:error, :bad_request, "Invalid file path"}