Get image width and height from the decoder (#9)

* Get image width and height from the decoder

* Remove unnecessary fields from state

* Add stream format validation

README.md

@@ -15,7 +15,7 @@ The package can be installed by adding `membrane_vpx_plugin` to your list of dep
 ```elixir
 def deps do
   [
-    {:membrane_vpx_plugin, "~> 0.2.0"}
+    {:membrane_vpx_plugin, "~> 0.3.0"}
   ]
 end
 ```

c_src/membrane_vpx_plugin/vpx_decoder.c

@@ -47,19 +47,19 @@ void get_raw_frame_from_image(vpx_image_t *img, UnifexPayload *raw_frame) {
   convert_between_image_and_raw_frame(img, raw_frame, IMAGE_TO_RAW_FRAME);
 }
 
-void alloc_output_frame(UnifexEnv *env, const vpx_image_t *img, UnifexPayload **output_frame) {
-  *output_frame = unifex_alloc(sizeof(UnifexPayload));
-  unifex_payload_alloc(env, UNIFEX_PAYLOAD_BINARY, get_image_byte_size(img), *output_frame);
+void alloc_output_frame(UnifexEnv *env, const vpx_image_t *img, decoded_frame *output_frame) {
+  output_frame->payload = unifex_alloc(sizeof(UnifexPayload));
+  unifex_payload_alloc(env, UNIFEX_PAYLOAD_BINARY, get_image_byte_size(img), output_frame->payload);
 }
 
-void free_payloads(UnifexPayload **payloads, unsigned int payloads_cnt) {
+void free_frames(decoded_frame *output_frames, unsigned int payloads_cnt) {
   for (unsigned int i = 0; i < payloads_cnt; i++) {
-    if (payloads[i] != NULL) {
-      unifex_payload_release(payloads[i]);
-      unifex_free(payloads[i]);
+    if (output_frames[i].payload != NULL) {
+      unifex_payload_release(output_frames[i].payload);
+      unifex_free(output_frames[i].payload);
     }
   }
-  unifex_free(payloads);
+  unifex_free(output_frames);
 }
 
 PixelFormat get_pixel_format_from_image(vpx_image_t *img) {
@@ -82,9 +82,8 @@ PixelFormat get_pixel_format_from_image(vpx_image_t *img) {
 UNIFEX_TERM decode_frame(UnifexEnv *env, UnifexPayload *frame, State *state) {
   vpx_codec_iter_t iter = NULL;
   vpx_image_t *img = NULL;
-  PixelFormat pixel_format = PIXEL_FORMAT_I420;
   unsigned int frames_cnt = 0, allocated_frames = 1;
-  UnifexPayload **output_frames = unifex_alloc(allocated_frames * sizeof(UnifexPayload *));
+  decoded_frame *output_frames = unifex_alloc(allocated_frames * sizeof(decoded_frame));
 
   if (vpx_codec_decode(&state->codec_context, frame->data, frame->size, NULL, 0)) {
     return result_error(
@@ -95,18 +94,21 @@ UNIFEX_TERM decode_frame(UnifexEnv *env, UnifexPayload *frame, State *state) {
   while ((img = vpx_codec_get_frame(&state->codec_context, &iter)) != NULL) {
     if (frames_cnt >= allocated_frames) {
       allocated_frames *= 2;
-      output_frames = unifex_realloc(output_frames, allocated_frames * sizeof(*output_frames));
+      output_frames = unifex_realloc(output_frames, allocated_frames * sizeof(decoded_frame));
     }
 
     alloc_output_frame(env, img, &output_frames[frames_cnt]);
-    get_raw_frame_from_image(img, output_frames[frames_cnt]);
-    pixel_format = get_pixel_format_from_image(img);
+
+    get_raw_frame_from_image(img, output_frames[frames_cnt].payload);
+    output_frames[frames_cnt].pixel_format = get_pixel_format_from_image(img);
+    output_frames[frames_cnt].width = img->d_w;
+    output_frames[frames_cnt].height = img->d_h;
     frames_cnt++;
   }
 
-  UNIFEX_TERM result = decode_frame_result_ok(env, output_frames, frames_cnt, pixel_format);
+  UNIFEX_TERM result = decode_frame_result_ok(env, output_frames, frames_cnt);
 
-  free_payloads(output_frames, frames_cnt);
+  free_frames(output_frames, frames_cnt);
 
   return result;
 }

c_src/membrane_vpx_plugin/vpx_decoder.spec.exs

@@ -6,10 +6,16 @@ type codec :: :vp8 | :vp9
 
 type pixel_format :: :I420 | :I422 | :I444 | :NV12 | :YV12
 
+type decoded_frame :: %DecodedFrame{
+       payload: payload,
+       pixel_format: pixel_format,
+       width: unsigned,
+       height: unsigned
+     }
+
 spec create(codec) :: {:ok :: label, state} | {:error :: label, reason :: atom}
 
 spec decode_frame(payload, state) ::
-       {:ok :: label, frames :: [payload], pixel_format :: pixel_format}
-       | {:error :: label, reason :: atom}
+       {:ok :: label, frames :: [decoded_frame]} | {:error :: label, reason :: atom}
 
 dirty :cpu, [:create, :decode_frame]

lib/membrane_vpx/decoder/vp8_decoder.ex

@@ -6,21 +6,7 @@ defmodule Membrane.VP8.Decoder do
 
   alias Membrane.{VP8, VPx}
 
-  def_options width: [
-                spec: pos_integer() | nil,
-                default: nil,
-                description: """
-                Width of a frame, needed if not provided with stream format. If it's not specified either in this option or the stream format, the element will crash.
-                """
-              ],
-              height: [
-                spec: pos_integer() | nil,
-                default: nil,
-                description: """
-                Height of a frame, needed if not provided with stream format. If it's not specified either in this option or the stream format, the element will crash.
-                """
-              ],
-              framerate: [
+  def_options framerate: [
                 spec: {non_neg_integer(), pos_integer()} | nil,
                 default: nil,
                 description: """

lib/membrane_vpx/decoder/vp9_decoder.ex

@@ -6,21 +6,7 @@ defmodule Membrane.VP9.Decoder do
 
   alias Membrane.{VP9, VPx}
 
-  def_options width: [
-                spec: pos_integer() | nil,
-                default: nil,
-                description: """
-                Width of a frame, needed if not provided with stream format. If it's not specified either in this option or the stream format, the element will crash.
-                """
-              ],
-              height: [
-                spec: pos_integer() | nil,
-                default: nil,
-                description: """
-                Height of a frame, needed if not provided with stream format. If it's not specified either in this option or the stream format, the element will crash.
-                """
-              ],
-              framerate: [
+  def_options framerate: [
                 spec: {non_neg_integer(), pos_integer()} | nil,
                 default: nil,
                 description: """

lib/membrane_vpx/decoder/vpx_decoder.ex

@@ -1,6 +1,7 @@
 defmodule Membrane.VPx.Decoder do
   @moduledoc false
 
+  require Membrane.Logger
   alias Membrane.{Buffer, RawVideo, RemoteStream, VP8, VP9}
   alias Membrane.Element.CallbackContext
   alias Membrane.VPx.Decoder.Native
@@ -10,13 +11,11 @@ defmodule Membrane.VPx.Decoder do
 
     @type t :: %__MODULE__{
             codec: :vp8 | :vp9,
-            width: pos_integer() | nil,
-            height: pos_integer() | nil,
             framerate: {pos_integer(), pos_integer()} | nil,
             decoder_ref: reference() | nil
           }
 
-    @enforce_keys [:codec, :width, :height, :framerate]
+    @enforce_keys [:codec, :framerate]
     defstruct @enforce_keys ++
                 [
                   decoder_ref: nil
@@ -28,12 +27,7 @@ defmodule Membrane.VPx.Decoder do
   @spec handle_init(CallbackContext.t(), VP8.Decoder.t() | VP9.Decoder.t(), :vp8 | :vp9) ::
           callback_return()
   def handle_init(_ctx, opts, codec) do
-    state_fields =
-      opts
-      |> Map.take([:width, :height, :framerate])
-      |> Map.put(:codec, codec)
-
-    {[], struct(State, state_fields)}
+    {[], %State{framerate: opts.framerate, codec: codec}}
   end
 
   @spec handle_setup(CallbackContext.t(), State.t()) :: callback_return()
@@ -51,51 +45,50 @@ defmodule Membrane.VPx.Decoder do
   @spec handle_buffer(:input, Membrane.Buffer.t(), CallbackContext.t(), State.t()) ::
           callback_return()
   def handle_buffer(:input, %Buffer{payload: payload, pts: pts}, ctx, state) do
-    {:ok, [decoded_frame], pixel_format} = Native.decode_frame(payload, state.decoder_ref)
+    {:ok, [decoded_frame]} = Native.decode_frame(payload, state.decoder_ref)
+
+    new_stream_format = %RawVideo{
+      width: decoded_frame.width,
+      height: decoded_frame.height,
+      framerate: state.framerate,
+      pixel_format: decoded_frame.pixel_format,
+      aligned: true
+    }
 
     stream_format_action =
-      if ctx.pads.output.stream_format == nil do
-        output_stream_format =
-          get_output_stream_format(ctx.pads.input.stream_format, pixel_format, state)
+      if new_stream_format != ctx.pads.output.stream_format do
+        validate_stream_formats(ctx.pads.input.stream_format, new_stream_format)
 
-        [stream_format: {:output, output_stream_format}]
+        [stream_format: {:output, new_stream_format}]
       else
         []
       end
 
-    {stream_format_action ++ [buffer: {:output, %Buffer{payload: decoded_frame, pts: pts}}],
-     state}
+    {stream_format_action ++
+       [buffer: {:output, %Buffer{payload: decoded_frame.payload, pts: pts}}], state}
   end
 
-  @spec get_output_stream_format(
-          RemoteStream.t() | VP8.t() | VP9.t(),
-          RawVideo.pixel_format(),
-          State.t()
-        ) :: RawVideo.t()
-  defp get_output_stream_format(input_stream_format, pixel_format, state) do
-    {width, height, framerate} =
-      case input_stream_format do
-        %RemoteStream{} ->
-          {
-            state.width || raise("Width not provided"),
-            state.height || raise("Height not provided"),
-            state.framerate
-          }
-
-        %{width: width, height: height} ->
-          {
-            width,
-            height,
-            state.framerate
-          }
-      end
-
-    %RawVideo{
-      width: width,
-      height: height,
-      framerate: framerate,
-      pixel_format: pixel_format,
-      aligned: true
-    }
+  @spec validate_stream_formats(RemoteStream.t() | VP8.t() | VP9.t(), RawVideo.t()) ::
+          :ok
+  defp validate_stream_formats(input_stream_format, output_stream_format) do
+    case input_stream_format do
+      %RemoteStream{} ->
+        :ok
+
+      %{width: width, height: height} ->
+        if width != output_stream_format.width do
+          Membrane.Logger.warning(
+            "Image width specified in stream format: #{inspect(width)} differs from the real image width: #{inspect(output_stream_format.width)}, using the actual value."
+          )
+        end
+
+        if height != output_stream_format.height do
+          Membrane.Logger.warning(
+            "Image height specified in stream format: #{inspect(height)} differs from the real image height: #{inspect(output_stream_format.height)}, using the actual value."
+          )
+        end
+
+        :ok
+    end
   end
 end

lib/membrane_vpx/encoder/vpx_encoder.ex

@@ -17,6 +17,8 @@ defmodule Membrane.VPx.Encoder do
           rc_target_bitrate: pos_integer()
         }
 
+  @type encoded_frame :: %{payload: binary(), pts: non_neg_integer(), is_keyframe: boolean()}
+
   defmodule State do
     @moduledoc false
 
@@ -39,11 +41,8 @@ defmodule Membrane.VPx.Encoder do
 
   @type callback_return :: {[Membrane.Element.Action.t()], State.t()}
 
-  @type encoded_frame :: %{payload: binary(), pts: non_neg_integer(), is_keyframe: boolean()}
-
   @spec handle_init(CallbackContext.t(), VP8.Encoder.t() | VP9.Encoder.t(), :vp8 | :vp9) ::
           callback_return()
-
   def handle_init(_ctx, opts, codec) do
     state = %State{
       codec: codec,

mix.exs

@@ -1,7 +1,7 @@
 defmodule Membrane.VPx.Plugin.Mixfile do
   use Mix.Project
 
-  @version "0.2.0"
+  @version "0.3.0"
   @github_url "https://github.com/membraneframework/membrane_vpx_plugin"
 
   def project do