-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
LibGfx/FLIC: Add a FLIC decoder #24518
base: master
Are you sure you want to change the base?
Conversation
ec1a11f
to
d5978de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing seems wrong at the global scope, but still there are a lot of comments
@@ -4,4 +4,4 @@ Executable=/bin/ImageViewer | |||
Category=Gra&phics | |||
|
|||
[Launcher] | |||
FileTypes=bmp,dds,gif,ico,iff,jb2,jbig2,jp2,jpeg,jpg,jpx,jxl,lbm,pbm,pgm,png,ppm,qoi,tga,tiff,tif,tvg | |||
FileTypes=bmp,dds,fli,flc,gif,ico,iff,jb2,jbig2,jp2,jpeg,jpg,jpx,jxl,lbm,pbm,pgm,png,ppm,qoi,tga,tiff,tif,tvg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:redlinus:
Please keep that list sorted
* SPDX-License-Identifier: BSD-2-Clause | ||
*/ | ||
|
||
#include <AK/Memory.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't think that you're using that
struct Format { | ||
enum : u16 { | ||
FLI = 0xAF11, | ||
FLC = 0xAF12, | ||
}; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just invented enum class
!
enum class Format : u16 {
FLI = 0xAF11,
FLC = 0xAF12,
}
}; | ||
}; | ||
|
||
struct ChunkType { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, please use enum class
}; | ||
|
||
struct FLICFrameDescriptor { | ||
Color color_map[256]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use Array
s rather than raw C-arrays
} | ||
} | ||
break; | ||
} | ||
|
||
default: { | ||
dbgln_if(FLIC_DEBUG, "Unknown main chunk {:#x}", chunk->type); | ||
dbgln_if(FLIC_DEBUG, "Unknown subchunk {:#x}", chunk->type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixup
@@ -233,8 +257,7 @@ static ErrorOr<void> read_subchunk(FLICLoadingContext& context) | |||
// Some FLI encoders save the wrong size on the subchunk header. | |||
// For example in FLI_COPY subchunks: let's make sure the size is is enough to hold | |||
// an uncompressed frame. | |||
size_t data_size = type == 0x10 ? AK::max(size - chunk_header_size, context.width * context.height) : (u16)size; | |||
dbgln_if(FLIC_DEBUG, "Using subchunk size {}", data_size); | |||
size_t data_size = type == ChunkType::FLI_COPY ? AK::max(size - chunk_header_size, context.width * context.height) : (u16)size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixup
for (auto i = 0; i < num_chunks; ++i) | ||
TRY(read_subchunk(context)); | ||
dbgln_if(FLIC_DEBUG, "Added subchunk offset={}", context.stream.offset()); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixup
x += type; | ||
case ChunkType::FLI_BRUN: { | ||
for (size_t y = 0; y < context.height; y++) { | ||
auto x = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using auto
to deduce an int is quite the overhead, use u32
or whatever is the most appropriate.
auto x = 0; | ||
// first byte was the packets count, but FLC variant can encode more than 256 bytes | ||
// so ignore it and use the frame's width instead | ||
TRY(stream->seek(1, SeekMode::FromCurrentPosition)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer a read rather than an seek.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
Please don't close: I intend to fix the problems. |
FLIC refers to FLI/FLC files which were generated by MS-DOS application Autodesk Animator. The animation format was quite popular in the mid-nineties.
serenityos-fli.mov
This PR only adds support for very basic FLI files which were limited to 320x200 and 64 colours. The format was extended with support for higher resolutions and more colours and was renamed FLC.
More PRs will follow to add support for missing FLI and FLC chunks.