Skip to content

Commit

Permalink
Address more small review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@maryla-uc
maryla-uc committed Aug 18, 2023
1 parent 2091fbc commit 23efb59
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 105 deletions.
2 changes: 1 addition & 1 deletion src/read.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3609,7 +3609,7 @@ static avifBool avifTilesCanBeDecodedWithSameCodecInstance(avifDecoderData * dat
{
if (data->color.tileCount == 1 && (data->alpha.tileCount == 1 || data->gainMap.tileCount == 1)) {
// Single tile image with single tile alpha/gain map plane. In this case each tile needs its own decoder since the planes will be
// "stolen". Stealing either the color or the alpha (or gain map) plane will invalidate the other one when decode is called the second
// "stolen". Stealing either the color or the alpha (or gain map) plane will invalidate the other ones when decode is called the second
// (or third) time.
return AVIF_FALSE;
}
Expand Down
58 changes: 23 additions & 35 deletions src/write.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,29 +551,23 @@ static avifBool avifEncoderDetectChanges(const avifEncoder * encoder, avifEncode
}

// Same as 'avifEncoderWriteColorProperties' but for the colr nclx box only.
static void avifEncoderWriteNclxProperty(avifRWStream * outputStream,
static void avifEncoderWriteNclxProperty(avifRWStream * dedupStream,
avifRWStream * outputStream,
const avifImage * imageMetadata,
struct ipmaArray * ipma,
avifItemPropertyDedup * dedup)
{
avifRWStream * s = outputStream;
if (dedup) {
assert(ipma);
// Use the dedup's temporary stream for box writes
s = &dedup->s;
}

if (dedup) {
avifItemPropertyDedupStart(dedup);
}
avifBoxMarker colr = avifRWStreamWriteBox(s, "colr", AVIF_BOX_SIZE_TBD);
avifRWStreamWriteChars(s, "nclx", 4); // unsigned int(32) colour_type;
avifRWStreamWriteU16(s, imageMetadata->colorPrimaries); // unsigned int(16) colour_primaries;
avifRWStreamWriteU16(s, imageMetadata->transferCharacteristics); // unsigned int(16) transfer_characteristics;
avifRWStreamWriteU16(s, imageMetadata->matrixCoefficients); // unsigned int(16) matrix_coefficients;
avifRWStreamWriteU8(s, (imageMetadata->yuvRange == AVIF_RANGE_FULL) ? 0x80 : 0); // unsigned int(1) full_range_flag;
// unsigned int(7) reserved = 0;
avifRWStreamFinishBox(s, colr);
avifBoxMarker colr = avifRWStreamWriteBox(dedupStream, "colr", AVIF_BOX_SIZE_TBD);
avifRWStreamWriteChars(dedupStream, "nclx", 4); // unsigned int(32) colour_type;
avifRWStreamWriteU16(dedupStream, imageMetadata->colorPrimaries); // unsigned int(16) colour_primaries;
avifRWStreamWriteU16(dedupStream, imageMetadata->transferCharacteristics); // unsigned int(16) transfer_characteristics;
avifRWStreamWriteU16(dedupStream, imageMetadata->matrixCoefficients); // unsigned int(16) matrix_coefficients;
avifRWStreamWriteU8(dedupStream, (imageMetadata->yuvRange == AVIF_RANGE_FULL) ? 0x80 : 0); // unsigned int(1) full_range_flag;
// unsigned int(7) reserved = 0;
avifRWStreamFinishBox(dedupStream, colr);
if (dedup) {
ipmaPush(ipma, avifItemPropertyDedupFinish(dedup, outputStream), AVIF_FALSE);
}
Expand Down Expand Up @@ -629,7 +623,7 @@ static void avifEncoderWriteColorProperties(avifRWStream * outputStream,

// HEIF 6.5.5.1, from Amendment 3 allows multiple colr boxes: "at most one for a given value of colour type"
// Therefore, *always* writing an nclx box, even if an a prof box was already written above.
avifEncoderWriteNclxProperty(outputStream, imageMetadata, ipma, dedup);
avifEncoderWriteNclxProperty(dedupStream, outputStream, imageMetadata, ipma, dedup);

avifEncoderWriteExtendedColorProperties(dedupStream, outputStream, imageMetadata, ipma, dedup);
}
Expand Down Expand Up @@ -712,27 +706,21 @@ static void avifEncoderWriteExtendedColorProperties(avifRWStream * dedupStream,
}

// Same as 'avifEncoderWriteColorProperties' but for properties related to High Dynamic Range only.
static void avifEncoderWriteHDRProperties(avifRWStream * outputStream,
static void avifEncoderWriteHDRProperties(avifRWStream * dedupStream,
avifRWStream * outputStream,
const avifImage * imageMetadata,
struct ipmaArray * ipma,
avifItemPropertyDedup * dedup)
{
avifRWStream * s = outputStream;
if (dedup) {
assert(ipma);
// Use the dedup's temporary stream for box writes
s = &dedup->s;
}

// Write Content Light Level Information, if present
if (imageMetadata->clli.maxCLL || imageMetadata->clli.maxPALL) {
if (dedup) {
avifItemPropertyDedupStart(dedup);
}
avifBoxMarker clli = avifRWStreamWriteBox(s, "clli", AVIF_BOX_SIZE_TBD);
avifRWStreamWriteU16(s, imageMetadata->clli.maxCLL); // unsigned int(16) max_content_light_level;
avifRWStreamWriteU16(s, imageMetadata->clli.maxPALL); // unsigned int(16) max_pic_average_light_level;
avifRWStreamFinishBox(s, clli);
avifBoxMarker clli = avifRWStreamWriteBox(dedupStream, "clli", AVIF_BOX_SIZE_TBD);
avifRWStreamWriteU16(dedupStream, imageMetadata->clli.maxCLL); // unsigned int(16) max_content_light_level;
avifRWStreamWriteU16(dedupStream, imageMetadata->clli.maxPALL); // unsigned int(16) max_pic_average_light_level;
avifRWStreamFinishBox(dedupStream, clli);
if (dedup) {
ipmaPush(ipma, avifItemPropertyDedupFinish(dedup, outputStream), AVIF_FALSE);
}
Expand Down Expand Up @@ -2019,14 +2007,14 @@ avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output)

avifEncoderWriteColorProperties(&s, itemMetadata, &item->ipma, dedup);
if (!isToneMappedImage) {
avifEncoderWriteHDRProperties(&s, itemMetadata, &item->ipma, dedup);
avifEncoderWriteHDRProperties(&dedup->s, &s, itemMetadata, &item->ipma, dedup);
}
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
} else if (item->itemCategory == AVIF_ITEM_GAIN_MAP) {
// Gain map specific properties

// Write just the colr nclx box
avifEncoderWriteNclxProperty(&s, itemMetadata, &item->ipma, dedup);
avifEncoderWriteNclxProperty(&dedup->s, &s, itemMetadata, &item->ipma, dedup);
#endif
}

Expand All @@ -2036,7 +2024,7 @@ avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output)
// Technically, the gain map is not an HDR image, but in the API, this is the most convenient
// place to put this data.

avifEncoderWriteHDRProperties(&s, imageMetadata->gainMap.image, &item->ipma, dedup);
avifEncoderWriteHDRProperties(&dedup->s, &s, imageMetadata->gainMap.image, &item->ipma, dedup);
}
#endif

Expand Down Expand Up @@ -2297,7 +2285,7 @@ avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output)
writeConfigBox(&s, &item->av1C, encoder->data->configPropName);
if (item->itemCategory == AVIF_ITEM_COLOR) {
avifEncoderWriteColorProperties(&s, imageMetadata, NULL, NULL);
avifEncoderWriteHDRProperties(&s, imageMetadata, NULL, NULL);
avifEncoderWriteHDRProperties(&s, &s, imageMetadata, NULL, NULL);
}

avifBoxMarker ccst = avifRWStreamWriteFullBox(&s, "ccst", AVIF_BOX_SIZE_TBD, 0, 0);
Expand Down Expand Up @@ -2433,10 +2421,10 @@ avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output)
}
avifBool isAlphaOrGainMap = item->itemCategory == AVIF_ITEM_ALPHA;
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
isAlphaOrGainMap = isAlphaOrGainMap || item->itemCategory == AVIF_ITEM_GAIN_MAP;
isAlphaOrGainMap = isAlphaOrGainMap || item->itemCategory == AVIF_ITEM_GAIN_MAP || !memcmp(item->type, "tmap", 4);
#endif
if (alphaAndGainMapPass != isAlphaOrGainMap) {
// only process alpha payloads when alphaPass is true
// only process alpha/gain map payloads when alphaAndGainMapPass is true
continue;
}

Expand Down
5 changes: 5 additions & 0 deletions tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ if(AVIF_ENABLE_GTEST OR AVIF_BUILD_APPS)
set(CMAKE_CXX_STANDARD 14)
add_library(aviftest_helpers OBJECT gtest/aviftest_helpers.cc)
target_link_libraries(aviftest_helpers avif_apps)
target_link_libraries(aviftest_helpers avif_internal)
endif()

################################################################################
Expand Down Expand Up @@ -292,6 +293,10 @@ if(AVIF_CODEC_AVM)
avifallocationtest avifbasictest avifchangesettingtest avifcllitest avifgridapitest avifincrtest avifiostatstest
avifmetadatatest avifprogressivetest avify4mtest PROPERTIES DISABLED True
)

if(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
set_tests_properties(avifgainmaptest PROPERTIES DISABLED True)
endif()
endif()

if(AVIF_BUILD_APPS)
Expand Down
94 changes: 25 additions & 69 deletions tests/gtest/avifgainmaptest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "avif/internal.h"
#include "aviftest_helpers.h"
#include "gtest/gtest.h"

namespace libavif {
namespace {

Expand Down Expand Up @@ -70,8 +71,8 @@ TEST(GainMapTest, EncodeDecodeBaseImageSdr) {
ASSERT_NE(gain_map, nullptr);
testutil::FillImageGradient(gain_map.get());
gain_map->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_FCC;
// While attached to the gain map, this represents the clli information of the
// tone mapped image.
// Even though this is attached to the gain map, it represents the clli
// information of the tone mapped image.
gain_map->clli.maxCLL = 10;
gain_map->clli.maxPALL = 5;

Expand Down Expand Up @@ -169,45 +170,6 @@ TEST(GainMapTest, EncodeDecodeBaseImageHdr) {
// .write(reinterpret_cast<char*>(encoded.data), encoded.size);
}

avifResult MergeGrid(int grid_cols, int grid_rows,
const std::vector<const avifImage*>& cells,
avifImage* merged) {
const uint32_t tile_width = cells[0]->width;
const uint32_t tile_height = cells[0]->height;
const uint32_t grid_width =
(grid_cols - 1) * tile_width + cells.back()->width;
const uint32_t grid_height =
(grid_rows - 1) * tile_height + cells.back()->height;

testutil::AvifImagePtr view(avifImageCreateEmpty(), avifImageDestroy);
if (!view) {
return AVIF_RESULT_OUT_OF_MEMORY;
}
avifCropRect rect = {};
for (int j = 0; j < grid_rows; ++j) {
rect.x = 0;
for (int i = 0; i < grid_cols; ++i) {
const avifImage* image = cells[j * grid_cols + i];
rect.width = image->width;
rect.height = image->height;
avifResult result = avifImageSetViewRect(view.get(), merged, &rect);
if (result != AVIF_RESULT_OK) {
return result;
}
avifImageCopySamples(/*dstImage=*/view.get(), image, AVIF_PLANES_ALL);
assert(!view->imageOwnsYUVPlanes);
rect.x += rect.width;
}
rect.y += rect.height;
}

if ((rect.x != grid_width) || (rect.y != grid_height)) {
return AVIF_RESULT_UNKNOWN_ERROR;
}

return AVIF_RESULT_OK;
}

TEST(GainMapTest, EncodeDecodeGrid) {
std::vector<testutil::AvifImagePtr> cells;
std::vector<const avifImage*> cell_ptrs;
Expand Down Expand Up @@ -265,17 +227,17 @@ TEST(GainMapTest, EncodeDecodeGrid) {
testutil::AvifImagePtr merged = testutil::CreateImage(
static_cast<int>(decoded->width), static_cast<int>(decoded->height),
decoded->depth, decoded->yuvFormat, AVIF_PLANES_ALL);
ASSERT_EQ(MergeGrid(kGridCols, kGridRows, cell_ptrs, merged.get()),
ASSERT_EQ(testutil::MergeGrid(kGridCols, kGridRows, cell_ptrs, merged.get()),
AVIF_RESULT_OK);

testutil::AvifImagePtr merged_gain_map =
testutil::CreateImage(static_cast<int>(decoded->gainMap.image->width),
static_cast<int>(decoded->gainMap.image->height),
decoded->gainMap.image->depth,
decoded->gainMap.image->yuvFormat, AVIF_PLANES_YUV);
ASSERT_EQ(
MergeGrid(kGridCols, kGridRows, gain_map_ptrs, merged_gain_map.get()),
AVIF_RESULT_OK);
ASSERT_EQ(testutil::MergeGrid(kGridCols, kGridRows, gain_map_ptrs,
merged_gain_map.get()),
AVIF_RESULT_OK);

// Verify that the input and decoded images are close.
ASSERT_GT(testutil::GetPsnr(*merged, *decoded), 40.0);
Expand Down Expand Up @@ -357,39 +319,33 @@ TEST(GainMapTest, InvalidGrid) {
}

TEST(GainMapTest, SequenceNotSupported) {
std::vector<testutil::AvifImagePtr> frames;
constexpr int kNumFrames = 4;

for (int i = 0; i < kNumFrames; ++i) {
testutil::AvifImagePtr image =
testutil::CreateImage(/*width=*/64, /*height=*/100, /*depth=*/10,
AVIF_PIXEL_FORMAT_YUV444, AVIF_PLANES_ALL);
ASSERT_NE(image, nullptr);
image->transferCharacteristics =
AVIF_TRANSFER_CHARACTERISTICS_SMPTE2084; // PQ
testutil::FillImageGradient(image.get());
testutil::AvifImagePtr gain_map =
testutil::CreateImage(/*width=*/64, /*height=*/100, /*depth=*/8,
AVIF_PIXEL_FORMAT_YUV420, AVIF_PLANES_YUV);
ASSERT_NE(gain_map, nullptr);
testutil::FillImageGradient(gain_map.get());
// 'image' now owns the gain map.
image->gainMap.image = gain_map.release();

frames.push_back(std::move(image));
}
testutil::AvifImagePtr image =
testutil::CreateImage(/*width=*/64, /*height=*/100, /*depth=*/10,
AVIF_PIXEL_FORMAT_YUV444, AVIF_PLANES_ALL);
ASSERT_NE(image, nullptr);
image->transferCharacteristics =
AVIF_TRANSFER_CHARACTERISTICS_SMPTE2084; // PQ
testutil::FillImageGradient(image.get());
testutil::AvifImagePtr gain_map =
testutil::CreateImage(/*width=*/64, /*height=*/100, /*depth=*/8,
AVIF_PIXEL_FORMAT_YUV420, AVIF_PLANES_YUV);
ASSERT_NE(gain_map, nullptr);
testutil::FillImageGradient(gain_map.get());
// 'image' now owns the gain map.
image->gainMap.image = gain_map.release();

testutil::AvifEncoderPtr encoder(avifEncoderCreate(), avifEncoderDestroy);
ASSERT_NE(encoder, nullptr);
testutil::AvifRwData encoded;
// Add a first frame.
avifResult result =
avifEncoderAddImage(encoder.get(), frames[0].get(),
avifEncoderAddImage(encoder.get(), image.get(),
/*durationInTimescales=*/2, AVIF_ADD_IMAGE_FLAG_NONE);
ASSERT_EQ(result, AVIF_RESULT_OK)
<< avifResultToString(result) << " " << encoder->diag.error;

// Add a second frame.
result =
avifEncoderAddImage(encoder.get(), frames[1].get(),
avifEncoderAddImage(encoder.get(), image.get(),
/*durationInTimescales=*/2, AVIF_ADD_IMAGE_FLAG_NONE);
// Image sequences with gain maps are not supported.
ASSERT_EQ(result, AVIF_RESULT_NOT_IMPLEMENTED)
Expand Down
4 changes: 4 additions & 0 deletions tests/gtest/avifgridapitest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ avifResult EncodeDecodeGrid(const std::vector<std::vector<Cell>>& cell_rows,
testutil::AvifImagePtr grid = testutil::CreateImage(
static_cast<int>(image->width), static_cast<int>(image->height),
/*depth=*/8, yuv_format, AVIF_PLANES_ALL);
const int num_rows = (int)cell_rows.size();
const int num_cols = (int)cell_rows[0].size();
AVIF_CHECKRES(testutil::MergeGrid(num_cols, num_rows, cell_images, grid.get()));

testutil::AvifImagePtr view(avifImageCreateEmpty(), avifImageDestroy);
if (!view) {
return AVIF_RESULT_OUT_OF_MEMORY;
Expand Down
50 changes: 50 additions & 0 deletions tests/gtest/aviftest_helpers.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <vector>

#include "avif/avif.h"
#include "avif/internal.h"
#include "avifpng.h"
#include "avifutil.h"

Expand Down Expand Up @@ -348,6 +349,55 @@ bool AreImagesEqual(const avifRGBImage& image1, const avifRGBImage& image2) {
return true;
}

avifResult MergeGrid(int grid_cols, int grid_rows,
const std::vector<AvifImagePtr>& cells,
avifImage* merged) {
std::vector<const avifImage*> ptrs(cells.size());
for (size_t i = 0; i < cells.size(); ++i) {
ptrs[i] = cells[i].get();
}
return MergeGrid(grid_cols, grid_rows, ptrs, merged);
}

avifResult MergeGrid(int grid_cols, int grid_rows,
const std::vector<const avifImage*>& cells,
avifImage* merged) {
const uint32_t tile_width = cells[0]->width;
const uint32_t tile_height = cells[0]->height;
const uint32_t grid_width =
(grid_cols - 1) * tile_width + cells.back()->width;
const uint32_t grid_height =
(grid_rows - 1) * tile_height + cells.back()->height;

testutil::AvifImagePtr view(avifImageCreateEmpty(), avifImageDestroy);
if (!view) {
return AVIF_RESULT_OUT_OF_MEMORY;
}
avifCropRect rect = {};
for (int j = 0; j < grid_rows; ++j) {
rect.x = 0;
for (int i = 0; i < grid_cols; ++i) {
const avifImage* image = cells[j * grid_cols + i];
rect.width = image->width;
rect.height = image->height;
avifResult result = avifImageSetViewRect(view.get(), merged, &rect);
if (result != AVIF_RESULT_OK) {
return result;
}
avifImageCopySamples(/*dstImage=*/view.get(), image, AVIF_PLANES_ALL);
assert(!view->imageOwnsYUVPlanes);
rect.x += rect.width;
}
rect.y += rect.height;
}

if ((rect.x != grid_width) || (rect.y != grid_height)) {
return AVIF_RESULT_UNKNOWN_ERROR;
}

return AVIF_RESULT_OK;
}

//------------------------------------------------------------------------------

AvifImagePtr ReadImage(const char* folder_path, const char* file_name,
Expand Down
Loading

0 comments on commit 23efb59

Please sign in to comment.