Closed Mis1eader-dev closed 6 months ago
Hello @Mis1eader-dev , thanks for the pr!
I have some doubts as to why the current likely_formats does not meet the requirements.
If we pass a single format such as {kFormatBmp}
to it, it should behave the same as very_likely_format
.
Hello @xiaozhuai , this PR allows it to hint to the actual file extension, and if it doesn't match, then it looks in likely_formats, and if they don't meet, it can either say it failed or start looking into the rest of the formats.
My use case for this is this: User uploads a .png file, I'll tell imageinfo that it is very likely a png file, and if it isn't, then in likely_formats I have a list of formats that I want to support, namely png and jpg, which then it checks for jpg, and if it doesn't match that too, it returns false.
Basically it is saying I know the user uploaded a png file, try validating it with the png checker, and if it fails, start checking for the formats I want to support, and if it is within none of them, I want to reject the upload.
My use case for this is this: User uploads a .png file, I'll tell imageinfo that it is very likely a png file, and if it isn't, then in likely_formats I have a list of formats that I want to support, namely png and jpg, which then it checks for jpg, and if it doesn't match that too, it returns false.
You can pass {kFormatPng, kFormatJpeg}
to likely_formats, and pass true
to must_be_one_of_likely_formats
.
Does this fit your needs?
It doesn't fit in because the library will end up checking all formats in likely_formats
, in which case let's say we supported different formats alphabetically, and the file extension the user has uploaded starts with Z, in this case the library starts checking them one by one, with no hint as to what the actual file extension is.
If the application wants to support 10 formats, and the user uploads a file which lies in the 10th format, it ends up checking 9 other formats before it hits the format that it very likely was to begin with.
API wise this PR doesn't break compatibility with the old code, and only has the overhead of 1 added if statement for programs using the old API.
If the application wants to support 10 formats, and the user uploads a file which lies in the 10th format, it ends up checking 9 other formats before it hits the format that it very likely was to begin with.
The like_formats
is order-sensitive. So the use case can be fit in.
// Suppose we got a file, and the extension is 'png'
std::string extension = "png";
// All supported formats (likely_formats), it will check one by one, respect the order
std::vector<imageinfo::Format> supported_formats = {
imageinfo::kFormatJpeg,
imageinfo::kFormatPng,
imageinfo::kFormatBmp,
imageinfo::kFormatTiff
};
std::vector<imageinfo::Format> likely_formats;
likely_formats.reserve(supported_formats.size());
std::unordered_map<std::string, imageinfo::Format> extension_to_format = {
{"jpg", imageinfo::kFormatJpeg},
{"jpeg", imageinfo::kFormatJpeg},
{"png", imageinfo::kFormatPng},
{"bmp", imageinfo::kFormatBmp},
{"tiff", imageinfo::kFormatTiff}
};
if (extension_to_format.find(extension) != extension_to_format.end()) {
// If the extension is in the map, we put it to the first place
// The most_likely_format here is actually the very_likely_format that you said
auto most_likely_format = extension_to_format[extension];
likely_formats.emplace_back(most_likely_format);
// Then we put the rest of the supported formats to the list
for (auto format : supported_formats) {
if (format != most_likely_format) {
likely_formats.emplace_back(format);
}
}
} else {
// If the extension is not in the map, we put the supported formats to the list
likely_formats = supported_formats;
}
auto info = imageinfo::parse<imageinfo::FilePathReader>(file, likely_formats, true);
Thank you for providing an example, however, this is expensive in a server application, having to construct a likely_formats
vector every time an image is uploaded.
The PR allows the library user to create a constant likely_formats
with all desired supported formats, then at runtime it passes in the very_likely_format to be checked first before hitting the rest of likely_formats.
I'm alright with if this use case does not fit the library's vision, it is to make it feature rich.
The PR allows the library user to create a constant likely_formats with all desired supported formats, then at runtime it passes in the very_likely_format to be checked first before hitting the rest of likely_formats.
I see, but I'm worried that it will cause confusion for the users. Anyway, I will consider this feature. BTW, please fix the conflict, and the CI code style check.
If you didn't install clang-format-17, it's ok to let me fix the code style for your PR : )
I fixed merge conflicts, however, my distro only supports clang-format up to 15, I'd appreciate it if you format the code
I rename very_likely_format
to most_likely_format
.
Now this pr looks good to me.
Thanks again for the pr @Mis1eader-dev : )
Keeps compatibility with the older API.
Allows the programmer to give a hint to the library to try parsing a certain format that it is sure to a good extent is going to be true.
Closes #7