Skip to content

Commit

Permalink
Make sure the gain map metadata is towards the start of the file. (#2479
Browse files Browse the repository at this point in the history
)

The 'tmap' item data containing the gain map metadata was mistakenly being put at the end of the file.
  • Loading branch information
maryla-uc authored Oct 17, 2024
1 parent 2c36aed commit 048488e
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 47 deletions.
7 changes: 4 additions & 3 deletions src/write.c
Original file line number Diff line number Diff line change
Expand Up @@ -2195,8 +2195,8 @@ static avifResult avifEncoderWriteMediaDataBox(avifEncoder * encoder,
const size_t mdatStartOffset = avifRWStreamOffset(s);
for (uint32_t itemPasses = 0; itemPasses < 3; ++itemPasses) {
// Use multiple passes to pack in the following order:
// * Pass 0: metadata (Exif/XMP)
// * Pass 1: alpha, gain map (AV1)
// * Pass 0: metadata (Exif/XMP/gain map metadata)
// * Pass 1: alpha, gain map image (AV1)
// * Pass 2: all other item data (AV1 color)
//
// See here for the discussion on alpha coming before color:
Expand All @@ -2215,7 +2215,8 @@ static avifResult avifEncoderWriteMediaDataBox(avifEncoder * encoder,
// this item has nothing for the mdat box
continue;
}
const avifBool isMetadata = !memcmp(item->type, "mime", 4) || !memcmp(item->type, "Exif", 4);
const avifBool isMetadata = !memcmp(item->type, "mime", 4) || !memcmp(item->type, "Exif", 4) ||
!memcmp(item->type, "tmap", 4);
if (metadataPass != isMetadata) {
// only process metadata (XMP/Exif) payloads when metadataPass is true
continue;
Expand Down
5 changes: 4 additions & 1 deletion tests/gtest/avif_fuzztest_dec_incr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,10 @@ void DecodeIncr(const std::string& arbitrary_bytes, bool is_persistent,
const uint32_t max_cell_height = reference->height;
const avifResult result = DecodeIncrementally(
encoded_data, decoder.get(), is_persistent, give_size_hint,
use_nth_image_api, *reference, max_cell_height);
use_nth_image_api, *reference, max_cell_height,
/*enable_fine_incremental_check=*/false,
/*expect_whole_file_read=*/true,
/*expect_parse_success_from_partial_file=*/false);
// The result does not matter, as long as we do not crash.
(void)result;
}
Expand Down
4 changes: 3 additions & 1 deletion tests/gtest/avif_fuzztest_enc_dec_incr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ void EncodeDecodeGridValid(ImagePtr image, EncoderPtr encoder,

const avifResult decode_result = DecodeNonIncrementallyAndIncrementally(
encoded_data, decoder.get(), is_encoded_data_persistent,
give_size_hint_to_decoder, /*use_nth_image_api=*/true, cell_height);
give_size_hint_to_decoder, /*use_nth_image_api=*/true, cell_height,
/*enable_fine_incremental_check=*/false, /*expect_whole_file_read=*/true,
/*expect_parse_success_from_partial_file=*/false);
ASSERT_EQ(decode_result, AVIF_RESULT_OK) << avifResultToString(decode_result);
}

Expand Down
3 changes: 2 additions & 1 deletion tests/gtest/avif_fuzztest_enc_dec_incr_experimental.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ void EncodeDecodeGridValid(ImagePtr image, EncoderPtr encoder,
const avifResult decode_result = DecodeNonIncrementallyAndIncrementally(
encoded_data, decoder.get(), is_encoded_data_persistent,
give_size_hint_to_decoder, /*use_nth_image_api=*/true, cell_height,
/*enable_fine_incremental_check=*/false, expect_whole_file_read);
/*enable_fine_incremental_check=*/false, expect_whole_file_read,
/*expect_parse_success_from_partial_file=*/false);
ASSERT_EQ(decode_result, AVIF_RESULT_OK) << avifResultToString(decode_result);
}

Expand Down
8 changes: 5 additions & 3 deletions tests/gtest/avifgainmaptest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -344,17 +344,18 @@ TEST(GainMapTest, EncodeDecodeGrid) {
constexpr int kCellHeight = 200;

for (int i = 0; i < kGridCols * kGridRows; ++i) {
const int gradient_offset = i * 10;
ImagePtr image =
testutil::CreateImage(kCellWidth, kCellHeight, /*depth=*/10,
AVIF_PIXEL_FORMAT_YUV444, AVIF_PLANES_ALL);
ASSERT_NE(image, nullptr);
image->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_PQ;
testutil::FillImageGradient(image.get());
testutil::FillImageGradient(image.get(), gradient_offset);
ImagePtr gain_map =
testutil::CreateImage(kCellWidth / 2, kCellHeight / 2, /*depth=*/8,
AVIF_PIXEL_FORMAT_YUV420, AVIF_PLANES_YUV);
ASSERT_NE(gain_map, nullptr);
testutil::FillImageGradient(gain_map.get());
testutil::FillImageGradient(gain_map.get(), gradient_offset);
// 'image' now owns the gain map.
image->gainMap = avifGainMapCreate();
ASSERT_NE(image->gainMap, nullptr);
Expand Down Expand Up @@ -422,7 +423,8 @@ TEST(GainMapTest, EncodeDecodeGrid) {
/*is_persistent=*/true, /*give_size_hint=*/true,
/*use_nth_image_api=*/false, kCellHeight,
/*enable_fine_incremental_check=*/true),
AVIF_RESULT_OK);
AVIF_RESULT_OK)
<< decoder->diag.error;

// Uncomment the following to save the encoded image as an AVIF file.
// std::ofstream("/tmp/avifgainmaptest_grid.avif", std::ios::binary)
Expand Down
70 changes: 47 additions & 23 deletions tests/gtest/avifincrtest_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ void ComparePartialYuva(const avifImage& image1, const avifImage& image2,
// byte_count were given to the decoder, for an image of height rows, split into
// cells of cell_height rows.
uint32_t GetMinDecodedRowCount(uint32_t height, uint32_t cell_height,
bool has_alpha, size_t available_byte_count,
size_t byte_count,
bool has_alpha, bool has_gain_map,
size_t available_byte_count, size_t byte_count,
bool enable_fine_incremental_check) {
// The whole image should be available when the full input is.
if (available_byte_count >= byte_count) {
Expand All @@ -104,15 +104,17 @@ uint32_t GetMinDecodedRowCount(uint32_t height, uint32_t cell_height,
}
available_byte_count -= 500;
byte_count -= 500;
// Alpha, if any, is assumed to be located before the other planes and to
// represent at most 50% of the payload.
if (has_alpha) {
if (available_byte_count <= (byte_count / 2)) {
return 0;
}
available_byte_count -= byte_count / 2;
byte_count -= byte_count / 2;
// Extra planes (alpha, gain map), if any, are assumed to be located before
// the color planes. It's assumed that each extra planes is at most
// total_size / (1 + num_extra_planes).
const int num_extra_planes = (has_alpha ? 1 : 0) + (has_gain_map ? 1 : 0);
const size_t max_size_of_extra_planes = static_cast<size_t>(
(byte_count / (num_extra_planes + 1)) * num_extra_planes);
if (available_byte_count <= max_size_of_extra_planes) {
return 0;
}
available_byte_count -= max_size_of_extra_planes;
byte_count -= max_size_of_extra_planes;
// Linearly map the input availability ratio to the decoded row ratio.
const uint32_t min_decoded_cell_row_count = static_cast<uint32_t>(
(height / cell_height) * available_byte_count / byte_count);
Expand Down Expand Up @@ -295,7 +297,8 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
bool give_size_hint, bool use_nth_image_api,
const avifImage& reference, uint32_t cell_height,
bool enable_fine_incremental_check,
bool expect_whole_file_read) {
bool expect_whole_file_read,
bool expect_parse_success_from_partial_file) {
// AVIF cells are at least 64 pixels tall.
if (cell_height != reference.height) {
AVIF_CHECKERR(cell_height >= 64u, AVIF_RESULT_INVALID_ARGUMENT);
Expand Down Expand Up @@ -332,6 +335,15 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
data.available.size = std::min(data.available.size + step, data.full_size);
parse_result = avifDecoderParse(decoder);
}
if (data.available.size == data.full_size &&
expect_parse_success_from_partial_file) {
// Can happen if the data is in 'idat', or if some metadata is at the end of
// the file. But ideally this should be avoided.
printf(
"ERROR: had to provide the whole file for avifDecoderParse() to "
"succeed\n");
AVIF_CHECKERR(false, AVIF_RESULT_INVALID_ARGUMENT);
}
AVIF_CHECKRES(parse_result);

// Decoding is incremental.
Expand All @@ -344,16 +356,29 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
std::cerr << (use_nth_image_api ? "avifDecoderNthImage(0)"
: "avifDecoderNextImage()")
<< " returned WAITING_ON_IO instead of OK";
return AVIF_RESULT_INVALID_ARGUMENT;
AVIF_CHECKERR(false, AVIF_RESULT_INVALID_ARGUMENT);
}
const uint32_t decoded_row_count = avifDecoderDecodedRowCount(decoder);
AVIF_CHECKERR(decoded_row_count >= previously_decoded_row_count,
AVIF_RESULT_INVALID_ARGUMENT);
if (decoded_row_count < previously_decoded_row_count) {
printf("ERROR: decoded row count decreased from %d to %d\n",
previously_decoded_row_count, decoded_row_count);
AVIF_CHECKERR(false, AVIF_RESULT_INVALID_ARGUMENT);
}
bool has_gain_map = false;
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
has_gain_map = (reference.gainMap != nullptr);
#endif
const uint32_t min_decoded_row_count = GetMinDecodedRowCount(
reference.height, cell_height, reference.alphaPlane != nullptr,
data.available.size, data.full_size, enable_fine_incremental_check);
AVIF_CHECKERR(decoded_row_count >= min_decoded_row_count,
AVIF_RESULT_INVALID_ARGUMENT);
has_gain_map, data.available.size, data.full_size,
enable_fine_incremental_check);
if (decoded_row_count < min_decoded_row_count) {
printf(
"ERROR: expected to have decoded at least %d rows with %zu available "
"bytes, but only %d were decoded\n",
min_decoded_row_count, data.available.size, decoded_row_count);
AVIF_CHECKERR(false, AVIF_RESULT_INVALID_ARGUMENT);
}
ComparePartialYuva(reference, *decoder->image, decoded_row_count);

previously_decoded_row_count = decoded_row_count;
Expand All @@ -376,19 +401,18 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
avifResult DecodeNonIncrementallyAndIncrementally(
const avifRWData& encoded_avif, avifDecoder* decoder, bool is_persistent,
bool give_size_hint, bool use_nth_image_api, uint32_t cell_height,
bool enable_fine_incremental_check, bool expect_whole_file_read) {
bool enable_fine_incremental_check, bool expect_whole_file_read,
bool expect_parse_success_from_partial_file) {
ImagePtr reference(avifImageCreateEmpty());
if (reference == nullptr) return AVIF_RESULT_INVALID_ARGUMENT;
decoder->allowIncremental = AVIF_FALSE;
if (avifDecoderReadMemory(decoder, reference.get(), encoded_avif.data,
encoded_avif.size) != AVIF_RESULT_OK) {
return AVIF_RESULT_INVALID_ARGUMENT;
}
AVIF_CHECKRES(avifDecoderReadMemory(decoder, reference.get(),
encoded_avif.data, encoded_avif.size));

const avifResult result = DecodeIncrementally(
encoded_avif, decoder, is_persistent, give_size_hint, use_nth_image_api,
*reference, cell_height, enable_fine_incremental_check,
expect_whole_file_read);
expect_whole_file_read, expect_parse_success_from_partial_file);
return result;
}

Expand Down
20 changes: 13 additions & 7 deletions tests/gtest/avifincrtest_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,26 @@ void EncodeRectAsIncremental(const avifImage& image, uint32_t width,
// incremental granularity. enable_fine_incremental_check checks that sample
// rows are gradually output when feeding more and more input bytes to the
// decoder.
avifResult DecodeIncrementally(const avifRWData& encoded_avif,
avifDecoder* decoder, bool is_persistent,
bool give_size_hint, bool use_nth_image_api,
const avifImage& reference, uint32_t cell_height,
bool enable_fine_incremental_check = false,
bool expect_whole_file_read = true);
// If 'expect_parse_success_from_partial_file' is true, avifDecoderParse
// should succeed before the whole file is available. Returns an error
// if avifDecoderParse fails until all the bytes are available.
// `expect_parse_success_from_partial_file` should be set to false if the file
// may be using 'idat' or may have some metadata at the end of the file.
avifResult DecodeIncrementally(
const avifRWData& encoded_avif, avifDecoder* decoder, bool is_persistent,
bool give_size_hint, bool use_nth_image_api, const avifImage& reference,
uint32_t cell_height, bool enable_fine_incremental_check = false,
bool expect_whole_file_read = true,
bool expect_parse_success_from_partial_file = true);

// Calls DecodeIncrementally() with the reference being a regular decoding of
// encoded_avif.
avifResult DecodeNonIncrementallyAndIncrementally(
const avifRWData& encoded_avif, avifDecoder* decoder, bool is_persistent,
bool give_size_hint, bool use_nth_image_api, uint32_t cell_height,
bool enable_fine_incremental_check = false,
bool expect_whole_file_read = true);
bool expect_whole_file_read = true,
bool expect_parse_success_from_partial_file = true);

} // namespace testutil
} // namespace avif
Expand Down
15 changes: 8 additions & 7 deletions tests/gtest/aviftest_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ void FillImagePlain(avifImage* image, const uint32_t yuva[4]) {
}
}

void FillImageGradient(avifImage* image) {
void FillImageGradient(avifImage* image, int offset) {
for (avifChannelIndex c :
{AVIF_CHAN_Y, AVIF_CHAN_U, AVIF_CHAN_V, AVIF_CHAN_A}) {
const uint32_t limitedRangeMin =
Expand All @@ -158,16 +158,17 @@ void FillImageGradient(avifImage* image) {
const uint32_t plane_height = avifImagePlaneHeight(image, c);
uint8_t* row = avifImagePlane(image, c);
const uint32_t row_bytes = avifImagePlaneRowBytes(image, c);
const uint32_t max_xy_sum = plane_width + plane_height - 2;
for (uint32_t y = 0; y < plane_height; ++y) {
for (uint32_t x = 0; x < plane_width; ++x) {
uint32_t value;
uint32_t value = (x + y + offset) % (max_xy_sum + 1);
if (image->yuvRange == AVIF_RANGE_FULL || c == AVIF_CHAN_A) {
value = (x + y) * ((1u << image->depth) - 1u) /
std::max(1u, plane_width + plane_height - 2);
value =
value * ((1u << image->depth) - 1u) / std::max(1u, max_xy_sum);
} else {
value = limitedRangeMin +
(x + y) * (limitedRangeMax - limitedRangeMin) /
std::max(1u, plane_width + plane_height - 2);
value = limitedRangeMin + value *
(limitedRangeMax - limitedRangeMin) /
std::max(1u, max_xy_sum);
}
if (avifImageUsesU16(image)) {
reinterpret_cast<uint16_t*>(row)[x] = static_cast<uint16_t>(value);
Expand Down
4 changes: 3 additions & 1 deletion tests/gtest/aviftest_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ ImagePtr CreateImage(int width, int height, int depth,

// Set all pixels of each plane of an image.
void FillImagePlain(avifImage* image, const uint32_t yuva[4]);
void FillImageGradient(avifImage* image);
// 'offset' is a value to spatially offset the gradient, useful to create
// distinct images.
void FillImageGradient(avifImage* image, int offset = 0);
void FillImageChannel(avifRGBImage* image, uint32_t channel_offset,
uint32_t value);

Expand Down

0 comments on commit 048488e

Please sign in to comment.