Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make av1 obu parsing optional #266

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

jech
Copy link
Contributor

@jech jech commented Apr 15, 2024

  • Make parsing of the OBU list in AV1Packet optional
  • Avoid usage of OBUElements in av1_packet_test.
  • Mark ReadFrames as being deprecated.

In Pion, the (*XXXPacket).Unmarshal operation is supposed to be
fast and perform no allocation, as it is sometimes used to check
just a single flag in the packet header. The AV1Packet
implementation breaks this property by parsing the whole list
of OBUs at Unmarshal time, even though it is likely to be unneeded.

Note that I strongly object to this solution, since it makes the
inefficient path (both in space and in time) the default, and thus
will make people write inefficient code. However, if that's the
cost to getting the brain-damange reverted, I won't scream too
loudly.

@jech jech mentioned this pull request Apr 15, 2024
@jech jech force-pushed the make-av1-obu-parsing-optional branch from 205d7df to 1e05b07 Compare April 15, 2024 16:58
Copy link

codecov bot commented Apr 15, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 86.19%. Comparing base (a663858) to head (bcbb9ea).

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

Files Patch % Lines
codecs/av1_packet.go 90.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
+ Coverage   86.05%   86.19%   +0.14%     
==========================================
  Files          22       22              
  Lines        1742     1746       +4     
==========================================
+ Hits         1499     1505       +6     
+ Misses        206      204       -2     
  Partials       37       37              
Flag Coverage Δ
go 86.19% <90.00%> (+0.14%) ⬆️
wasm 85.68% <85.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jech jech force-pushed the make-av1-obu-parsing-optional branch 3 times, most recently from 5eb687f to ed8f74e Compare April 15, 2024 17:07
@jech jech force-pushed the make-av1-obu-parsing-optional branch from ed8f74e to 4da5ba4 Compare April 15, 2024 19:34
@jech
Copy link
Contributor Author

jech commented Apr 15, 2024

I've just made the accessor into a method, so its presence can be tested by callers (by checking whether the packet implements a given interface). I see that H264 and H265 have the same problem, so I've made the name codec-agnostic.

@jech jech force-pushed the make-av1-obu-parsing-optional branch 2 times, most recently from 2c9f7bf to bcbb9ea Compare April 15, 2024 22:35
Make parsing of the OBU list in AV1Packet optional. This enables a
higher performance depacketizer, with a reduced feature set.
@Sean-Der Sean-Der force-pushed the make-av1-obu-parsing-optional branch from bcbb9ea to ee561cf Compare April 23, 2024 03:51
@Sean-Der Sean-Der merged commit aa48ccf into pion:master Apr 23, 2024
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants