Add bitmap support [WIP] #483
Conversation
|
Hi @dougtoppin / @evanwieren / @hyandell , @fisenkodv May I ask you guys to review this one? Thank you in advance |
| imageSignature === "89504E47" || | ||
| imageSignature === "FFD8FFDB" || | ||
| imageSignature === "FFD8FFE0" || | ||
| imageSignature === "FFD8FFEE" || | ||
| imageSignature === "FFD8FFE1" || | ||
| imageSignature === "52494646" || | ||
| imageSignature === "49492A00" || | ||
| imageSignature === "4D4D002A" |
There was a problem hiding this comment.
These constants are already defined, in the image-request.ts, why don't check only 424d here?
| import Jimp from "jimp"; | ||
| import { ImageHandlerError, StatusCodes } from "./lib"; | ||
|
|
||
| export class BitmapRequest { |
There was a problem hiding this comment.
This class mostly converter from BMP to PNG, IMO ImageConverter is more appropriate name, or you can get rid of it at all and move into image-request.ts, like
private convert(imageBuffer: Buffer): Promise<Buffer> {
// if image is BMP return converted into PNG buffer
// else do nothing and return `imageBuffer`
}| @@ -15,7 +15,8 @@ | |||
| "color": "4.2.3", | |||
| "color-name": "1.1.4", | |||
| "sharp": "^0.31.3", | |||
| "aws-sdk": "^2.1356.0" | |||
| "aws-sdk": "^2.1356.0", | |||
| "jimp": "^0.22.8" | |||
There was a problem hiding this comment.
This package brings a lot of dependencies, for BMP it uses https://github.com/shaozilee/bmp-js, please refer to https://github.com/jimp-dev/jimp/blob/245f1cf60e0167d4d733ef90e638428d065856b2/packages/type-bmp/package.json#L25 I think you could use bmp-js instead.
| read: jest.fn() | ||
| })); | ||
|
|
||
| describe('BitmapRequest', () => { |
There was a problem hiding this comment.
From my point of view, you should not consider it as a Request. Please create appropriate tests in
source/image-handler/test/image-handlersource/image-handler/test/image-request/infer-image-type.spec.ts
|
@ fisenkodv thanks for review that. Gonna adjust everything and let you know 馃憤 |
|
@coelhosandro Is this something that you are still interested in pursuing?
|
Issue #, if available:
We have stored in S3 images in bitmap format. Such files are not supported by the image handler. This patch aims to convert those files as PNG before processing the request
Description of changes:
Added a class to read/identify Bitmaps files using Jimp and convert those into PNG (which is supported by Sharp)
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.