Skip to content

make av1 obu parsing optional - #266

Merged
Sean-Der merged 1 commit into
pion:masterfrom
jech:make-av1-obu-parsing-optional
Apr 23, 2024
Merged

make av1 obu parsing optional#266
Sean-Der merged 1 commit into
pion:masterfrom
jech:make-av1-obu-parsing-optional

Conversation

@jech

@jech jech commented Apr 15, 2024

Copy link
Copy Markdown
Member
  • 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
@codecov

codecov Bot commented Apr 15, 2024

Copy link
Copy Markdown

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

jech commented Apr 15, 2024

Copy link
Copy Markdown
Member Author

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
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.

2 participants