nrkno / sofie-atem-connection

Sofie ATEM Connection: A Part of the Sofie TV Studio Automation System
https://github.com/nrkno/Sofie-TV-automation/
MIT License
129 stars 36 forks source link

perf: Add RLE encoding #149

Closed ianshade closed 1 year ago

ianshade commented 1 year ago
Julusian commented 1 year ago

Nice, this is one of things that I have been wondering about, but the benefit was too much of an unknown for me to be able to motivate.

I'll give this and your other PR a proper read next week.

Should we make RLE optional?

I think it would be good to make it opt out. Perhaps always disabled when running in single-threaded mode, as that 30-40ms of cpu time might be long enough for the connection to drop?
Making it opt-out gives a nice escape hatch in case it does cause connection issues, or for when you know that the RLE will do nothing (eg, source is from a jpeg).
I think this should be specified with an options object, so that we can add some other options later, like specifying the input format or output colourspace. (we currently assume its a RGBA buffer, and converting to 709)

Should we try WASM with SIMD for the color space conversion and RLE to squeeze out some more performance?

If we were to move the RGB->YUV into that too then WASM will probably be beneficial. The problem with WASM is that the pixels will need to be copied from a nodejs buffer into a WASM buffer, and the inverse on the way out, so if not careful it could be more costly.
I prototyped a similar conversion in my streamdeck library, which did halve the time of the buffer operations, but the numbers (for 72x72 pixels) were small enough to make the benefit questionable

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 90.47% and project coverage change: +0.05% :tada:

Comparison is base (3a331ff) 86.23% compared to head (bb9a1dc) 86.28%. Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #149 +/- ## ========================================== + Coverage 86.23% 86.28% +0.05% ========================================== Files 177 180 +3 Lines 5709 5813 +104 Branches 941 957 +16 ========================================== + Hits 4923 5016 +93 - Misses 764 775 +11 Partials 22 22 ``` | [Files Changed](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno) | Coverage Δ | | |---|---|---| | [src/atem.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2F0ZW0udHM=) | `20.24% <0.00%> (-0.01%)` | :arrow_down: | | [src/index.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2luZGV4LnRz) | `100.00% <ø> (ø)` | | | [src/dataTransfer/dataTransferUploadBuffer.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2RhdGFUcmFuc2Zlci9kYXRhVHJhbnNmZXJVcGxvYWRCdWZmZXIudHM=) | `75.00% <88.88%> (+2.65%)` | :arrow_up: | | [src/lib/atemUtil.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2xpYi9hdGVtVXRpbC50cw==) | `63.01% <95.00%> (+12.07%)` | :arrow_up: | | [src/dataTransfer/dataTransferUploadAudio.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2RhdGFUcmFuc2Zlci9kYXRhVHJhbnNmZXJVcGxvYWRBdWRpby50cw==) | `100.00% <100.00%> (ø)` | | | [src/dataTransfer/dataTransferUploadClip.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2RhdGFUcmFuc2Zlci9kYXRhVHJhbnNmZXJVcGxvYWRDbGlwLnRz) | `89.85% <100.00%> (+0.30%)` | :arrow_up: | | [src/dataTransfer/dataTransferUploadMacro.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2RhdGFUcmFuc2Zlci9kYXRhVHJhbnNmZXJVcGxvYWRNYWNyby50cw==) | `100.00% <100.00%> (ø)` | | | [...dataTransfer/dataTransferUploadMultiViewerLabel.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2RhdGFUcmFuc2Zlci9kYXRhVHJhbnNmZXJVcGxvYWRNdWx0aVZpZXdlckxhYmVsLnRz) | `100.00% <100.00%> (ø)` | | | [src/dataTransfer/dataTransferUploadStill.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2RhdGFUcmFuc2Zlci9kYXRhVHJhbnNmZXJVcGxvYWRTdGlsbC50cw==) | `100.00% <100.00%> (ø)` | | | [src/dataTransfer/index.ts](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno#diff-c3JjL2RhdGFUcmFuc2Zlci9pbmRleC50cw==) | `89.91% <100.00%> (+1.12%)` | :arrow_up: | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/nrkno/sofie-atem-connection/pull/149/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nrkno)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Julusian commented 1 year ago

I have made this opt-out, and this is now released.

I also started playing around with doing the RGBA->YUVA conversion in a native library in #152, we could look at moving the RLE to that too later on, once/if that gets merged