Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
TimonLukas opened this issue Feb 25, 2024 · 2 comments
Open

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

TimonLukas opened this issue Feb 25, 2024 · 2 comments

Comments

@TimonLukas
Copy link

TimonLukas commented Feb 25, 2024

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
Copy link
Owner

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

{ colorMode: 'rgba', xFlip: this.xyFlip, yFlip: this.xyFlip },
, 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
Copy link
Author

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?

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

No branches or pull requests

2 participants