Fix a bug where timestamps didn't take into account dropped packets (#63

)

* Fix a bug where pts were wrong in case of drops

* Start accounting for dropped packets in duration

README.md

@@ -15,7 +15,7 @@ The package can be installed by adding `membrane_opus_plugin` to your list of de
 ```elixir
 def deps do
   [
-    {:membrane_opus_plugin, "~> 0.20.1"}
+    {:membrane_opus_plugin, "~> 0.20.2"}
   ]
 end
 ```

lib/membrane_opus/parser.ex

@@ -155,7 +155,7 @@ defmodule Membrane.Opus.Parser do
            FrameLengths.parse(frame_packing, data, state.input_delimitted?),
          expected_packet_size <- header_size + Enum.sum(frame_lengths) + padding_size,
          {:ok, raw_packet, rest} <- rest_of_packet(data, expected_packet_size) do
-      duration = elapsed_time(frame_lengths, frame_duration)
+      duration = packet_duration(frame_lengths, frame_duration)
 
       packet = %Buffer{
         pts: state.current_pts,
@@ -210,12 +210,12 @@ defmodule Membrane.Opus.Parser do
     end
   end
 
-  @spec elapsed_time(frame_lengths :: [non_neg_integer], frame_duration :: pos_integer) ::
-          elapsed_time :: Membrane.Time.non_neg()
-  defp elapsed_time(frame_lengths, frame_duration) do
-    # if a frame has length 0 it indicates a dropped frame and should not be
-    # included in this calc
-    present_frames = frame_lengths |> Enum.count(fn length -> length > 0 end)
-    present_frames * frame_duration
+  @spec packet_duration(
+          frame_lengths :: [non_neg_integer()],
+          frame_duration :: Membrane.Time.non_neg()
+        ) ::
+          duration :: Membrane.Time.non_neg()
+  defp packet_duration(frame_lengths, frame_duration) do
+    length(frame_lengths) * frame_duration
   end
 end

mix.exs

@@ -1,7 +1,7 @@
 defmodule Membrane.Opus.Plugin.Mixfile do
   use Mix.Project
 
-  @version "0.20.1"
+  @version "0.20.2"
   @github_url "https://github.com/membraneframework/membrane_opus_plugin"
 
   def project do

test/membrane_opus/parser_test.exs

@@ -16,7 +16,7 @@ defmodule Membrane.Opus.Parser.ParserTest do
       normal: <<4>>,
       delimited: <<4, 0>>,
       channels: 2,
-      duration: 0,
+      duration: 10 |> milliseconds(),
       pts: 0
     },
     %{
@@ -25,39 +25,39 @@ defmodule Membrane.Opus.Parser.ParserTest do
       delimited: <<121, 2, 0, 0, 0, 0>>,
       channels: 1,
       duration: 40 |> milliseconds(),
-      pts: 0
+      pts: 10 |> milliseconds()
     },
     %{
       desc: "code 2",
       normal: <<198, 1, 0, 0, 0, 0>>,
       delimited: <<198, 1, 3, 0, 0, 0, 0>>,
       channels: 2,
       duration: 5 |> milliseconds(),
-      pts: 40 |> milliseconds()
+      pts: 50 |> milliseconds()
     },
     %{
       desc: "code 3 cbr, no padding",
       normal: <<199, 3, 0, 0, 0>>,
       delimited: <<199, 3, 1, 0, 0, 0>>,
       channels: 2,
       duration: (2.5 * 3 * 1_000_000) |> trunc() |> nanoseconds(),
-      pts: 45 |> milliseconds()
+      pts: 55 |> milliseconds()
     },
     %{
       desc: "code 3 cbr, padding",
       normal: <<199, 67, 2, 0, 0, 0, 0, 0>>,
       delimited: <<199, 67, 2, 1, 0, 0, 0, 0, 0>>,
       channels: 2,
       duration: (2.5 * 3 * 1_000_000) |> trunc() |> nanoseconds(),
-      pts: (52.5 * 1_000_000) |> trunc() |> nanoseconds()
+      pts: (62.5 * 1_000_000) |> trunc() |> nanoseconds()
     },
     %{
       desc: "code 3 vbr, no padding",
       normal: <<199, 131, 1, 2, 0, 0, 0, 0>>,
       delimited: <<199, 131, 1, 2, 1, 0, 0, 0, 0>>,
       channels: 2,
       duration: (2.5 * 3 * 1_000_000) |> trunc() |> nanoseconds(),
-      pts: 60 |> milliseconds()
+      pts: 70 |> milliseconds()
     },
     %{
       desc: "code 3 vbr, no padding, long length",
@@ -85,7 +85,7 @@ defmodule Membrane.Opus.Parser.ParserTest do
           0, 0, 3, 3, 3>>,
       channels: 2,
       duration: (2.5 * 3 * 1_000_000) |> trunc() |> nanoseconds(),
-      pts: (67.5 * 1_000_000) |> trunc() |> nanoseconds()
+      pts: (77.5 * 1_000_000) |> trunc() |> nanoseconds()
     }
   ]
 
@@ -248,7 +248,7 @@ defmodule Membrane.Opus.Parser.ParserTest do
       assert_sink_buffer(pipeline, :sink, ^expected_buffer, 4000)
     end)
 
-    refute_sink_buffer(pipeline, :sink, 0)
+    refute_sink_buffer(pipeline, :sink, _, 0)
 
     @fixtures
     |> Enum.map(& &1.channels)