remove av1 obu parsing #265
Conversation
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. We remove OBU parsing from Unmarshal, to make the method consistent with the other implementations in Pion (for example, H264Packet does not parse the list of NALs at Unmarshal time). If OBU parsing is required, it should be provided as a user-callable function in codecs/av1/obu.
It used to depend on the now removed OBU parsing code.
|
Sorry about that @jech Could we make it opt-out? |
Yes, that's what I'm suggesting. I suggest adding a new function: so that callers can opt-in simply by calling it. I volunteer to do that and, if required, to redo the removed frame code to use that function. Do I have your agreement in principle? |
|
What do you think of adding a All the Unmarshal functions have the same problem. They keep getting expanded to add more functionality. |
It means that the default path is the inefficient behaviour, which has a bad smell. I'll do it, but only if you insist really strongly. |
|
Please see #266. Please read the last paragraph of the PR description, and please consider removing the brain-damange altogether. |
|
Replaced by #266 |
Remove OBU parsing from AV1Packet
In Pion, the 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.
We remove OBU parsing from Unmarshal, to make the method consistent
with the other implementations in Pion (for example, H264Packet
does not parse the list of NALs at Unmarshal time). We also remove the code in
codecs/frame, which depends on the OBU parsing.
If the code in codecs/frame is required, I volunteer to put it back in a way that
doesn't require parsing the whole list of OBUs at every call to Unmarshal.