mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2024-11-22 15:40:19 +00:00
LibRIFF+LibGfx/ISOBMFF: Make ChunkID (de)serialization self-consistent
Previously, ChunkID's from_big_endian_number() and as_big_endian_number() weren't inverses of each other. ChunkID::from_big_endian_number() used to take an u32 that contained `('f' << 24) | ('t' << 16) | ('y' << 8) | 'p'`, that is 'f', 't', 'y', 'p' in memory on big-endian and 'p', 'y', 't', 'f' on little-endian, and return a ChunkID for 'f', 't', 'y', 'p'. ChunkID::as_big_endian_number() used to return an u32 that for a ChunkID storing 'f', 't', 'y', 'p' was always 'f', 't', 'y', 'p' in memory on both little-endian and big-endian, that is it stored `('f' << 24) | ('t' << 16) | ('y' << 8) | 'p'` on big-endian and `('p' << 24) | ('y' << 16) | ('t' << 8) | 'f'` on little-endian. `ChunkID::from_big_endian_number(0x11223344).as_big_endian_number()` returned 0x44332211. This change makes the two methods self-consistent: they now take and return a u32 that always has the first ChunkID part in the highest bits of the u32 (`'f' << 24`), and so on. That also means they return a u32 that in-memory looks differently on big-endian and little-endian. Since that's normal for numbers, this also renames the two methods to just `from_number()` and `to_number()`. With the semantics cleared up, change the one use in ISOBMFF to read a BigEndian for chunk headers and brand codes. This has the effect of tags now being printed in the right order. Before: ```sh % Build/lagom/bin/isobmff ~/Downloads/sample1.jp2 Unknown Box (' Pj') [ 4 bytes ] ('pytf') (version = 0, flags = 0x0) - major_brand = ' 2pj' - minor_version = 0 - compatible_brands = { ' 2pj' } Unknown Box ('h2pj') [ 37 bytes ] Unknown Box ('fniu') [ 92 bytes ] Unknown Box (' lmx') [ 2736 bytes ] Unknown Box ('c2pj') [ 667336 bytes ] ``` After: ```sh % Build/lagom/bin/isobmff ~/Downloads/sample1.jp2 hmm 0x11223344 0x11223344 Unknown Box ('jP ') [ 4 bytes ] ('ftyp' ) (version = 0, flags = 0x0) - major_brand = 'jp2 ' - minor_version = 0 - compatible_brands = { 'jp2 ' } Unknown Box ('jp2h') [ 37 bytes ] Unknown Box ('uinf') [ 92 bytes ] Unknown Box ('xml ') [ 2736 bytes ] Unknown Box ('jp2c') [ 667336 bytes ] ```
This commit is contained in:
parent
b43092db46
commit
0d098211b7
Notes:
sideshowbarker
2024-07-17 09:49:33 +09:00
Author: https://github.com/nico Commit: https://github.com/SerenityOS/serenity/commit/0d098211b7 Pull-request: https://github.com/SerenityOS/serenity/pull/23678 Reviewed-by: https://github.com/kleinesfilmroellchen ✅
3 changed files with 18 additions and 10 deletions
|
@ -12,7 +12,7 @@ ErrorOr<BoxHeader> read_box_header(Stream& stream)
|
||||||
{
|
{
|
||||||
BoxHeader header;
|
BoxHeader header;
|
||||||
u64 total_size = TRY(stream.read_value<BigEndian<u32>>());
|
u64 total_size = TRY(stream.read_value<BigEndian<u32>>());
|
||||||
header.type = TRY(stream.read_value<BoxType>());
|
header.type = TRY(stream.read_value<BigEndian<BoxType>>());
|
||||||
|
|
||||||
u64 data_size_read = sizeof(u32) + sizeof(BoxType);
|
u64 data_size_read = sizeof(u32) + sizeof(BoxType);
|
||||||
|
|
||||||
|
@ -68,7 +68,7 @@ void UnknownBox::dump(String const& prepend) const
|
||||||
ErrorOr<void> FileTypeBox::read_from_stream(BoxStream& stream)
|
ErrorOr<void> FileTypeBox::read_from_stream(BoxStream& stream)
|
||||||
{
|
{
|
||||||
// unsigned int(32) major_brand;
|
// unsigned int(32) major_brand;
|
||||||
major_brand = TRY(stream.read_value<BrandIdentifier>());
|
major_brand = TRY(stream.read_value<BigEndian<BrandIdentifier>>());
|
||||||
// unsigned int(32) minor_version;
|
// unsigned int(32) minor_version;
|
||||||
minor_version = TRY(stream.read_value<BigEndian<u32>>());
|
minor_version = TRY(stream.read_value<BigEndian<u32>>());
|
||||||
|
|
||||||
|
@ -77,7 +77,7 @@ ErrorOr<void> FileTypeBox::read_from_stream(BoxStream& stream)
|
||||||
return Error::from_string_literal("FileTypeBox compatible_brands contains a partial brand");
|
return Error::from_string_literal("FileTypeBox compatible_brands contains a partial brand");
|
||||||
|
|
||||||
for (auto minor_brand_count = stream.remaining() / sizeof(BrandIdentifier); minor_brand_count > 0; minor_brand_count--)
|
for (auto minor_brand_count = stream.remaining() / sizeof(BrandIdentifier); minor_brand_count > 0; minor_brand_count--)
|
||||||
TRY(compatible_brands.try_append(TRY(stream.read_value<BrandIdentifier>())));
|
TRY(compatible_brands.try_append(TRY(stream.read_value<BigEndian<BrandIdentifier>>())));
|
||||||
|
|
||||||
return {};
|
return {};
|
||||||
}
|
}
|
||||||
|
|
|
@ -23,7 +23,7 @@ namespace Gfx::ISOBMFF {
|
||||||
enum class BoxType : u32 {
|
enum class BoxType : u32 {
|
||||||
None = 0,
|
None = 0,
|
||||||
|
|
||||||
#define ENUMERATE_ONE(box_name, box_4cc) box_name = RIFF::ChunkID(#box_4cc).as_big_endian_number(),
|
#define ENUMERATE_ONE(box_name, box_4cc) box_name = RIFF::ChunkID(#box_4cc).as_number(),
|
||||||
|
|
||||||
ENUMERATE_ALL()
|
ENUMERATE_ALL()
|
||||||
|
|
||||||
|
@ -54,7 +54,7 @@ static bool is_valid_box_type(BoxType type)
|
||||||
enum class BrandIdentifier : u32 {
|
enum class BrandIdentifier : u32 {
|
||||||
None = 0,
|
None = 0,
|
||||||
|
|
||||||
#define ENUMERATE_ONE(brand_4cc) brand_4cc = RIFF::ChunkID(#brand_4cc).as_big_endian_number(),
|
#define ENUMERATE_ONE(brand_4cc) brand_4cc = RIFF::ChunkID(#brand_4cc).as_number(),
|
||||||
|
|
||||||
ENUMERATE_ALL()
|
ENUMERATE_ALL()
|
||||||
|
|
||||||
|
@ -70,7 +70,7 @@ struct AK::Formatter<Gfx::ISOBMFF::BoxType> : Formatter<FormatString> {
|
||||||
ErrorOr<void> format(FormatBuilder& builder, Gfx::ISOBMFF::BoxType const& box_type)
|
ErrorOr<void> format(FormatBuilder& builder, Gfx::ISOBMFF::BoxType const& box_type)
|
||||||
{
|
{
|
||||||
StringView format_string = Gfx::ISOBMFF::is_valid_box_type(box_type) ? "({})"sv : "Unknown Box ({})"sv;
|
StringView format_string = Gfx::ISOBMFF::is_valid_box_type(box_type) ? "({})"sv : "Unknown Box ({})"sv;
|
||||||
return Formatter<FormatString>::format(builder, format_string, RIFF::ChunkID::from_big_endian_number(to_underlying(box_type)));
|
return Formatter<FormatString>::format(builder, format_string, RIFF::ChunkID::from_number(to_underlying(box_type)));
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -78,6 +78,6 @@ template<>
|
||||||
struct AK::Formatter<Gfx::ISOBMFF::BrandIdentifier> : Formatter<FormatString> {
|
struct AK::Formatter<Gfx::ISOBMFF::BrandIdentifier> : Formatter<FormatString> {
|
||||||
ErrorOr<void> format(FormatBuilder& builder, Gfx::ISOBMFF::BrandIdentifier const& brand_identifier)
|
ErrorOr<void> format(FormatBuilder& builder, Gfx::ISOBMFF::BrandIdentifier const& brand_identifier)
|
||||||
{
|
{
|
||||||
return Formatter<FormatString>::format(builder, "{}"sv, RIFF::ChunkID::from_big_endian_number(to_underlying(brand_identifier)));
|
return Formatter<FormatString>::format(builder, "{}"sv, RIFF::ChunkID::from_number(to_underlying(brand_identifier)));
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
|
@ -32,14 +32,22 @@ struct ChunkID {
|
||||||
constexpr ChunkID(ChunkID const&) = default;
|
constexpr ChunkID(ChunkID const&) = default;
|
||||||
constexpr ChunkID(ChunkID&&) = default;
|
constexpr ChunkID(ChunkID&&) = default;
|
||||||
constexpr ChunkID& operator=(ChunkID const&) = default;
|
constexpr ChunkID& operator=(ChunkID const&) = default;
|
||||||
static constexpr ChunkID from_big_endian_number(u32 number) { return bit_cast<Array<u8, chunk_id_size>>(AK::convert_between_host_and_big_endian(number)); }
|
static constexpr ChunkID from_number(u32 number)
|
||||||
|
{
|
||||||
|
return Array<u8, chunk_id_size> { {
|
||||||
|
static_cast<u8>((number >> 24) & 0xff),
|
||||||
|
static_cast<u8>((number >> 16) & 0xff),
|
||||||
|
static_cast<u8>((number >> 8) & 0xff),
|
||||||
|
static_cast<u8>(number & 0xff),
|
||||||
|
} };
|
||||||
|
}
|
||||||
|
|
||||||
static ErrorOr<ChunkID> read_from_stream(Stream& stream);
|
static ErrorOr<ChunkID> read_from_stream(Stream& stream);
|
||||||
|
|
||||||
StringView as_ascii_string() const;
|
StringView as_ascii_string() const;
|
||||||
constexpr u32 as_big_endian_number() const
|
constexpr u32 as_number() const
|
||||||
{
|
{
|
||||||
return AK::convert_between_host_and_big_endian((id_data[0] << 24) | (id_data[1] << 16) | (id_data[2] << 8) | id_data[3]);
|
return (id_data[0] << 24) | (id_data[1] << 16) | (id_data[2] << 8) | id_data[3];
|
||||||
}
|
}
|
||||||
|
|
||||||
bool operator==(ChunkID const&) const = default;
|
bool operator==(ChunkID const&) const = default;
|
||||||
|
|
Loading…
Reference in a new issue