Julusian / node-elgato-stream-deck

A Node.js library for interfacing with the Elgato Stream Deck. https://julusian.github.io/node-elgato-stream-deck/
https://www.npmjs.com/org/elgato-stream-deck
MIT License
163 stars 21 forks source link

Add option to directly supply JPEG buffers for LCD/keys #82

Open TimonLukas opened 9 months ago

TimonLukas commented 9 months ago

Hey there! Awesome library, thank you very much for your work!

I'm writing a volume mixer for Linux with the Streamdeck Plus, and have started optimization. Profiling showed me that a lot of time was spent in the conversion functions and JPEG encoding (already using jpeg-turbo). Since I'm using sharp for generating the frames, I can skip all the preparation and immediately supply the right JPEG buffer - this skips one full-size buffer allocation per draw call as well as a bunch of unnecessary JS-side array options.

Testing showed me that this slightly reduced CPU usage without impacting negatively impacting speed. To properly allow this, I propose adding a new type as a union to the options of the different fill methods:

interface FillImageOptions {
    format: 'rgb' | 'rgba' | 'bgr' | 'bgra'
}

interface FillRawOptions {
    raw: true,
    // or maybe:
    // jpeg: true,
} 

function fillEncoderLcd(index: EncoderIndex, imageBuffer: Buffer, sourceOptions: FillImageOptions | FillRawOptions): Promise<void>

If this is passed, all previous checks and preparations should be skipped, passing the imageBuffer directly as the byteBuffer to the functions responsible for generating writes. This would of course mean opting out all other options, but that should hopefully be fine with documentation :)

This would allow for user-side experimentation and optimization. What do you think?

Julusian commented 9 months ago

I'm not against adding this, but its not going to be nice to use. Different models require different orientations for the images, as well as some of the older ones expecting bitmaps not jpegs, so for an application to generate the images correctly will require knowledge of each model they want to support (and this can vary within hardware revisions of the same model too). Perhaps there should be a property on the class to describe this encoding too, so that this knowledge can be shared to applications like yours? Something like the value used in https://github.com/Julusian/node-elgato-stream-deck/blob/698f04fcc0e03dc20bca9be705c314415576b46e/packages/core/src/models/plus.ts#L186, but replacing colorMode with fomat: 'jpeg' | 'bmp'

I think it would be better to have:

interface FillRawOptions {
    format: 'raw',
} 

I'm not entirely sold on it being 'raw', but as this should also work for passing bitmaps for the older models I don't think it should be 'jpeg'

TimonLukas commented 9 months ago

I'm not against adding this, but its not going to be nice to use. Different models require different orientations for the images, as well as some of the older ones expecting bitmaps not jpegs, so for an application to generate the images correctly will require knowledge of each model they want to support (and this can vary within hardware revisions of the same model too). Perhaps there should be a property on the class to describe this encoding too, so that this knowledge can be shared to applications like yours?

That approach sounds great to me!

I'm not entirely sold on it being 'raw', but as this should also work for passing bitmaps for the older models I don't think it should be 'jpeg'

I considered raw because I'm trying to express "the library will not do further operations on the buffer" - maybe encoded or direct would represent that better?