API and Implementation Improvements

[Cassy343]

Primarily removes unnecessary use of dynamic dispatch, interior mutability, and async functions. As a result some public APIs have changed, generally making them more Rusty.

[k0nserv]

Overall this looks like a good improvement.

I'm curious to see how this will look when integrated in to the other webrtc-rs repositories. Are you creating sibling pull requests for this?

[codecov]

Codecov Report

Merging #27 (dac3f16) into main (2591e4f) will decrease coverage by 0.09%.
The diff coverage is 59.57%.

❗ Current head dac3f16 differs from pull request most recent head 66e1389. Consider uploading reports for the commit 66e1389 to get more accurate results

@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
- Coverage   28.26%   28.17%   -0.10%     
==========================================
  Files          16       16              
  Lines        4688     4643      -45     
  Branches     1153     1152       -1     
==========================================
- Hits         1325     1308      -17     
+ Misses       2954     2933      -21     
+ Partials      409      402       -7     

Impacted Files	Coverage Δ
src/packet/packet_test.rs	21.82% <0.00%> (-0.15%)	⬇️
src/error.rs	4.08% <35.71%> (+0.51%)	⬆️
src/sequence.rs	58.82% <61.53%> (+10.43%)	⬆️
src/packetizer/packetizer_test.rs	67.39% <75.00%> (+2.47%)	⬆️
src/codecs/vp9/vp9_test.rs	87.89% <100.00%> (-0.38%)	⬇️
src/lib.rs	6.51% <100.00%> (+0.03%)	⬆️
src/header.rs	60.82% <0.00%> (-0.52%)	⬇️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2591e4f...66e1389. Read the comment docs.

[Cassy343]

Will address requested changes and the CI failure later today.

My bandwidth is a little limited at the moment, so while I'll likely make some sibling PRs to other repos, I can't guarantee how soon/if that will happen. I made this PR since I needed this crate in a project I'm working on and wanted to see these changes rendered if possible.

[k0nserv]

My bandwidth is a little limited at the moment, so while I'll likely make some sibling PRs to other repos, I can't guarantee how soon/if that will happen. I made this PR since I needed this crate in a project I'm working on and wanted to see these changes rendered if possible.

The thing is, this project is directly consumed in the other webrtc-rs projects and as such merging it will require work in all upstream projects. I'm likewise strapped for bandwidth and won't have time to upstream these breaking changes.

I'd be reluctant to merge this until we at least know the scope of the upstream changes required and whether the changes would be problematic to integrate in those projects.

[k0nserv]

I'd be reluctant to merge this until we at least know the scope of the upstream changes required and whether the changes would be problematic to integrate in those projects.

Actually, thinking about this. This is still an improvement even if you or I don't have time to upstream the changes atm.

[Cassy343]

8b4116b should resolve all the feedback above. Let me know if you have any more questions/concerns.

Actually, thinking about this. This is still an improvement even if you or I don't have time to upstream the changes atm.

It's also worth bearing in mind that it's unlikely that an individual or group of individuals would have the spare time to synchronously rework the API of this whole ecosystem niche, so most changes will probably have to be done incrementally :)

[melekes]

🔥

[Cassy343]

So uh I'll fix the cargo format issue but not sure what to do about the fact that we hit an ICE in CI job

EDIT: looks like it's a known issue rust-lang/rust#99263

[k0nserv]

We have migrated this crate to the monorepo(webrtc-rs/webrtc) please re-open this pull request over there.