Skip to content

Commit

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

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.