elixir-webrtc / ex_webrtc

An Elixir implementation of the W3C WebRTC API
Apache License 2.0
363 stars 14 forks source link

Hide VP8 and Opus Payloader/Depayloader modules #154

Closed mickel8 closed 3 months ago

mickel8 commented 3 months ago

Sorry, I don't want to mess in your PR but I have a couple of observations:

An alternative approach would be to use Protocols. The API would look like this:

defprotocol ExWebRTC.RTP.Payloader do
  def payload(t(), data) 
end

defprotocol ExWebRTC.RTP.Depayloader do
  def depayload(t(), data)
end

defmodule ExWebRTC.RTP.Utils do
  alias ExWebRTC.RTPCodecParameters

  @spec new_payloader(RTPCodecParameters.t()) :: ExWebRTC.RTP.Payloader.t()
  def new_payloader(codec_params) do
  end

  @spec new_depayloader(RTPCodecParameters.t()) :: ExWebRTC.RTP.Depayloader.t()
  def new_depayloader(codec_params) do
  end
end

This way, user can use RTP utils to automatically find payloader/depayloader or implement their own payloade/depayloader and still use our protocols. This would be beneficial if we used those protocols internally, in PeerConnection but we don't right now.

Any thoughts?

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.52%. Comparing base (b142d04) to head (be6c244).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## sgfn/payloader-behaviour #154 +/- ## ========================================================= Coverage 88.51% 88.52% ========================================================= Files 38 38 Lines 1907 1908 +1 ========================================================= + Hits 1688 1689 +1 Misses 219 219 ``` | [Files](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc) | Coverage Δ | | |---|---|---| | [lib/ex\_webrtc/rtp/depayloader.ex](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?src=pr&el=tree&filepath=lib%2Fex_webrtc%2Frtp%2Fdepayloader.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc#diff-bGliL2V4X3dlYnJ0Yy9ydHAvZGVwYXlsb2FkZXIuZXg=) | `100.00% <100.00%> (ø)` | | | [lib/ex\_webrtc/rtp/opus/depayloader.ex](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?src=pr&el=tree&filepath=lib%2Fex_webrtc%2Frtp%2Fopus%2Fdepayloader.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc#diff-bGliL2V4X3dlYnJ0Yy9ydHAvb3B1cy9kZXBheWxvYWRlci5leA==) | `100.00% <ø> (ø)` | | | [lib/ex\_webrtc/rtp/opus/payloader.ex](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?src=pr&el=tree&filepath=lib%2Fex_webrtc%2Frtp%2Fopus%2Fpayloader.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc#diff-bGliL2V4X3dlYnJ0Yy9ydHAvb3B1cy9wYXlsb2FkZXIuZXg=) | `100.00% <ø> (ø)` | | | [lib/ex\_webrtc/rtp/payloader.ex](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?src=pr&el=tree&filepath=lib%2Fex_webrtc%2Frtp%2Fpayloader.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc#diff-bGliL2V4X3dlYnJ0Yy9ydHAvcGF5bG9hZGVyLmV4) | `100.00% <100.00%> (ø)` | | | [lib/ex\_webrtc/rtp/vp8/depayloader.ex](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?src=pr&el=tree&filepath=lib%2Fex_webrtc%2Frtp%2Fvp8%2Fdepayloader.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc#diff-bGliL2V4X3dlYnJ0Yy9ydHAvdnA4L2RlcGF5bG9hZGVyLmV4) | `77.27% <100.00%> (ø)` | | | [lib/ex\_webrtc/rtp/vp8/payload.ex](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?src=pr&el=tree&filepath=lib%2Fex_webrtc%2Frtp%2Fvp8%2Fpayload.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc#diff-bGliL2V4X3dlYnJ0Yy9ydHAvdnA4L3BheWxvYWQuZXg=) | `90.00% <ø> (ø)` | | | [lib/ex\_webrtc/rtp/vp8/payloader.ex](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?src=pr&el=tree&filepath=lib%2Fex_webrtc%2Frtp%2Fvp8%2Fpayloader.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc#diff-bGliL2V4X3dlYnJ0Yy9ydHAvdnA4L3BheWxvYWRlci5leA==) | `100.00% <ø> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc). Last update [b142d04...be6c244](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/154?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc).