Skip to content
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.

Sign up for GitHub

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

Check that the gain map max is >= gain map min. #2580

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Check that the gain map max is >= gain map min. #2580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/gainmap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,11 @@ avifResult avifGainMapValidateMetadata(const avifGainMap * gainMap, avifDiagnost
avifDiagnosticsPrintf(diag, "Per-channel denominator is 0 in gain map metadata");
return AVIF_RESULT_INVALID_ARGUMENT;
}
if ((int64_t)gainMap->gainMapMax[i].n * gainMap->gainMapMin[i].d <
(int64_t)gainMap->gainMapMin[i].n * gainMap->gainMapMax[i].d) {
avifDiagnosticsPrintf(diag, "Per-channel max is less than per-channel min in gain map metadata");
return AVIF_RESULT_INVALID_ARGUMENT;
}
if (gainMap->gainMapGamma[i].n == 0) {
avifDiagnosticsPrintf(diag, "Per-channel gamma is 0 in gain map metadata");
return AVIF_RESULT_INVALID_ARGUMENT;
Expand Down
4 changes: 2 additions & 2 deletions tests/gtest/avif_fuzztest_enc_dec.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ void EncodeDecodeValid(ImagePtr image, EncoderPtr encoder, DecoderPtr decoder) {
const avifResult encoder_result =
avifEncoderWrite(encoder.get(), image.get(), &encoded_data);
ASSERT_EQ(encoder_result, AVIF_RESULT_OK)
<< avifResultToString(encoder_result);
<< avifResultToString(encoder_result) << " " << encoder->diag.error;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: change " " to ": " so that the output looks like ERROR_CODE: error message.

Also in line 57.


const avifResult decoder_result = avifDecoderReadMemory(
decoder.get(), decoded_image.get(), encoded_data.data, encoded_data.size);
ASSERT_EQ(decoder_result, AVIF_RESULT_OK)
<< avifResultToString(decoder_result);
<< avifResultToString(decoder_result) << " " << decoder->diag.error;

EXPECT_EQ(decoded_image->width, image->width);
EXPECT_EQ(decoded_image->height, image->height);
Expand Down
21 changes: 10 additions & 11 deletions tests/gtest/avif_fuzztest_helpers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,10 @@ DecoderPtr AddGainMapOptionsToDecoder(
}

ImagePtr AddGainMapToImage(
ImagePtr image, ImagePtr gain_map, int32_t gain_map_min_n0,
int32_t gain_map_min_n1, int32_t gain_map_min_n2, uint32_t gain_map_min_d0,
uint32_t gain_map_min_d1, uint32_t gain_map_min_d2, int32_t gain_map_max_n0,
int32_t gain_map_max_n1, int32_t gain_map_max_n2, uint32_t gain_map_max_d0,
uint32_t gain_map_max_d1, uint32_t gain_map_max_d2,
ImagePtr image, ImagePtr gain_map,
const std::pair<avifSignedFraction, avifSignedFraction>& gain_map_min_max0,
const std::pair<avifSignedFraction, avifSignedFraction>& gain_map_min_max1,
const std::pair<avifSignedFraction, avifSignedFraction>& gain_map_min_max2,
uint32_t gain_map_gamma_n0, uint32_t gain_map_gamma_n1,
uint32_t gain_map_gamma_n2, uint32_t gain_map_gamma_d0,
uint32_t gain_map_gamma_d1, uint32_t gain_map_gamma_d2,
Expand All @@ -182,13 +181,13 @@ ImagePtr AddGainMapToImage(
image->gainMap = avifGainMapCreate();
image->gainMap->image = gain_map.release();

image->gainMap->gainMapMin[0] = {gain_map_min_n0, gain_map_min_d0};
image->gainMap->gainMapMin[1] = {gain_map_min_n1, gain_map_min_d1};
image->gainMap->gainMapMin[2] = {gain_map_min_n2, gain_map_min_d2};
image->gainMap->gainMapMin[0] = gain_map_min_max0.first;
image->gainMap->gainMapMin[1] = gain_map_min_max1.first;
image->gainMap->gainMapMin[2] = gain_map_min_max2.first;

image->gainMap->gainMapMax[0] = {gain_map_max_n0, gain_map_max_d0};
image->gainMap->gainMapMax[1] = {gain_map_max_n1, gain_map_max_d1};
image->gainMap->gainMapMax[2] = {gain_map_max_n2, gain_map_max_d2};
image->gainMap->gainMapMax[0] = gain_map_min_max0.second;
image->gainMap->gainMapMax[1] = gain_map_min_max1.second;
image->gainMap->gainMapMax[2] = gain_map_min_max2.second;

image->gainMap->gainMapGamma[0] = {gain_map_gamma_n0, gain_map_gamma_d0};
image->gainMap->gainMapGamma[1] = {gain_map_gamma_n1, gain_map_gamma_d1};
Expand Down
43 changes: 27 additions & 16 deletions tests/gtest/avif_fuzztest_helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <cstdint>
#include <cstdlib>
#include <limits>
#include <utility>
#include <vector>

#include "avif/avif.h"
Expand Down Expand Up @@ -167,13 +168,31 @@ inline auto ArbitraryAvifAnim() {
return fuzztest::OneOf(ArbitraryAvifAnim8b(), ArbitraryAvifAnim16b());
}

// TODO: Try StructOf<Metadata>(StructOf<uint32_t[3]>())?
// Generates two signed fractions where the first one is smaller than the second
// one.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the first one is smaller than or equal to the second one, right?

inline auto ArbitraryMinMaxSignedFraction() {
return fuzztest::FlatMap(
[](int32_t max_n, uint32_t max_d) {
return fuzztest::Map(
[max_n, max_d](int32_t min_n) {
// For simplicity, use the same denominator for both fractions.
// This does not cover all possible fractions but makes it easy
// to gurantee that the fraction is smaller.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: gurantee => guarantee

Add "first" before "fraction".

return std::pair<avifSignedFraction, avifSignedFraction>(
{min_n, max_d}, {max_n, max_d});
},
fuzztest::InRange<int32_t>(std::numeric_limits<int32_t>::min(),
max_n));
},
fuzztest::Arbitrary<int32_t>(),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: Use fuzztest::NonZero<uint32_t>() instead?

If this change is correct, make the same change to the other instances below. Or you can make these change in a separate pull request.

}

ImagePtr AddGainMapToImage(
ImagePtr image, ImagePtr gain_map, int32_t gain_map_min_n0,
int32_t gain_map_min_n1, int32_t gain_map_min_n2, uint32_t gain_map_min_d0,
uint32_t gain_map_min_d1, uint32_t gain_map_min_d2, int32_t gain_map_max_n0,
int32_t gain_map_max_n1, int32_t gain_map_max_n2, uint32_t gain_map_max_d0,
uint32_t gain_map_max_d1, uint32_t gain_map_max_d2,
ImagePtr image, ImagePtr gain_map,
const std::pair<avifSignedFraction, avifSignedFraction>& gain_map_min_max0,
const std::pair<avifSignedFraction, avifSignedFraction>& gain_map_min_max1,
const std::pair<avifSignedFraction, avifSignedFraction>& gain_map_min_max2,
uint32_t gain_map_gamma_n0, uint32_t gain_map_gamma_n1,
uint32_t gain_map_gamma_n2, uint32_t gain_map_gamma_d0,
uint32_t gain_map_gamma_d1, uint32_t gain_map_gamma_d2,
Expand All @@ -189,16 +208,8 @@ ImagePtr AddGainMapToImage(
inline auto ArbitraryAvifImageWithGainMap() {
return fuzztest::Map(
AddGainMapToImage, ArbitraryAvifImage(), ArbitraryAvifImage(),
fuzztest::Arbitrary<int32_t>(), fuzztest::Arbitrary<int32_t>(),
fuzztest::Arbitrary<int32_t>(),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
fuzztest::Arbitrary<int32_t>(), fuzztest::Arbitrary<int32_t>(),
fuzztest::Arbitrary<int32_t>(),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
ArbitraryMinMaxSignedFraction(), ArbitraryMinMaxSignedFraction(),
ArbitraryMinMaxSignedFraction(),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
fuzztest::InRange<uint32_t>(1, std::numeric_limits<uint32_t>::max()),
Expand Down
Loading