meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.25k stars 2.48k forks source link

pp-av1: use obuparse #3227

Closed tmatth closed 3 weeks ago

tmatth commented 1 year ago

WIP

Currently a discrepancy after ever "Detected new resolution", size is 1 less and header type is 2 instead of 6:

image

Upstream is here: https://github.com/dwbuiten/obuparse License is permissve (ISC)

lminiero commented 1 year ago

That looks like a lot of additional code, and I'm not sure if there's a reason for it: after all, the post-processor code mostly only cares about OBU when figuring out the resolution, and only in part for depacketization. For that, isn't our simpler code enough? Besides, to be honest I'm not sure I'm comfortable embedding a bunch of external code I'd then have to keep up to date with: I went through that with Duktape in the past and it was a mess, and in fact we changed that to an external dependency to be installed separately.

tmatth commented 1 year ago

That looks like a lot of additional code, and I'm not sure if there's a reason for it: after all, the post-processor code mostly only cares about OBU when figuring out the resolution, and only in part for depacketization. For that, isn't our simpler code enough? Besides, to be honest I'm not sure I'm comfortable embedding a bunch of external code I'd then have to keep up to date with: I went through that with Duktape in the past and it was a mess, and in fact we changed that to an external dependency to be installed separately.

@lminiero I marked this as draft as I'm not sure if it's worth merging it yet. I also want to validate the existing parsing.

lminiero commented 1 year ago

Ack, I'll wait for more feedback then, thanks for the clarification!

tmatth commented 3 weeks ago

I decided to drop this since the OBUParse use case isn't really the same fit as RTP AV1 bitstream parsing that pp-av1 needs.