Skip to content

Disable strict pixi requirement for libavif >= 0.9.1 #7253

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

Closed
wants to merge 4 commits into from
Closed

Disable strict pixi requirement for libavif >= 0.9.1 #7253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5889a4a
Disable strict pixi requirement for libavif >= 0.9.1
morsssss Jul 16, 2021
36698d2
Remove xfail from test on FreeBSD
morsssss Jul 20, 2021
89b277e
Use more compact AVIF_VERSION
morsssss Jul 26, 2021
d6226da
concatenate comment
morsssss Aug 6, 2021
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
68 changes: 68 additions & 0 deletions .vscode/launch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
{
// Use IntelliSense to learn about possible attributes.
// Hover to view descriptions of existing attributes.
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{
"name": "PHP aviftest",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/sapi/cli/php",
"args": ["../play/php/aviftest.php"],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
"environment": [],
"externalConsole": false,
"MIMode": "lldb"
},
{
"name": "getimagesize",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/sapi/cli/php",
"args": ["../play/php/getimagesize.php"],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
"environment": [],
"externalConsole": false,
"MIMode": "lldb"
},
{
"name": "imagecreatefromstring_avif",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/sapi/cli/php",
"args": ["ext/gd/tests/imagecreatefromstring_avif.phpt"],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
"environment": [],
"externalConsole": false,
"MIMode": "lldb"
},
{
"name": "Web-Pee",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/sapi/cli/php",
"args": ["ext/gd/tests/imagecreatefromstring_webp.phpt"],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
"environment": [],
"externalConsole": false,
"MIMode": "lldb"
},
{
"name": "avif_decode_encode",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/sapi/cli/php",
"args": ["ext/gd/tests/avif_decode_encode.phpt"],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
"environment": [],
"externalConsole": false,
"MIMode": "lldb"
}
]
}
3 changes: 3 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"C_Cpp.dimInactiveRegions": false
}
8 changes: 8 additions & 0 deletions ext/gd/libgd/gd_avif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,14 @@ gdImagePtr gdImageCreateFromAvifCtx (gdIOCtx *ctx)

decoder = avifDecoderCreate();

// Check if libavif version is >= 0.9.1
// If so, allow the PixelInformationProperty ('pixi') to be missing in AV1 image
// items. libheif v1.11.0 or older does not add the 'pixi' item property to
// AV1 image items. (This issue has been corrected in libheif v1.12.0.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to link with libheif instead of libavif?

Copy link
Contributor

Choose a reason for hiding this comment

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

got a test failure when libavif is build without libaom https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/23479

Copy link
Member

Choose a reason for hiding this comment

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

Is it due to just some minor variation of pixels? In that case we probably should use similarity.inc for the comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you use a different encoder than aom? If so, which did you use?

For libgd, we decided to use libaom.

About libheif - this library can certainly create AVIF images as well! For various reasons, we decided to use libavif in gd. So I ultimately optimized my logic for libavif. To use libheif instead, you'd need to rewrite gd_avif.c.

Copy link
Contributor

Choose a reason for hiding this comment

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

@morsssss thank you! I used to build libavif with aom and applied your patch - tests now passed on 0.9.2

Copy link
Contributor

Choose a reason for hiding this comment

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

Both the PR and bb9ef2b still shows failures on arm (guess it caused by 32-bits)

Copy link
Member

Choose a reason for hiding this comment

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

This test passes on 32bit Windows with libavif 0.9.0; so either it's only an issue with newer libavif, or not particularly 32bit related at all.

Copy link
Contributor

@andypost andypost Aug 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Still fails on armv7 and armhf using beta2 (libavif 0.9.2)

TEST 6120/15019 [ext/gd/tests/imagecreatefromstring_avif.phpt]
========DIFF========
002+ Frame size limit reduced from 268435456 to 67108864.
     int(250)
     int(375)
006+ Frame size limit reduced from 268435456 to 67108864.
     int(480)
     int(270)
========DONE========
FAIL imagecreatefromstring() - AVIF format [ext/gd/tests/imagecreatefromstring_avif.phpt]

TEST 5938/15019 [ext/gd/tests/avif_decode_encode.phpt]
========DIFF========
001+ Decoding AVIF image: Frame size limit reduced from 268435456 to 67108864.
002+ Frame size limit reduced from 268435456 to 67108864.
003+ ok
001- Decoding AVIF image: ok
     Default AVIF encoding: ok
     Encoding AVIF at quality 70: ok
     Encoding AVIF at quality 70 with speed 5: ok
--
     Encoding AVIF with illegal quality: ok
     Encoding AVIF with illegal speed: ok
     Encoding AVIF losslessly... ok
012+ Frame size limit reduced from 268435456 to 67108864.
     How many pixels are different in the two images? 0
========DONE========
FAIL avif decoding/encoding tests [ext/gd/tests/avif_decode_encode.phpt]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andypost , please see the thread below.

Copy link
Contributor

Choose a reason for hiding this comment

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

@morsssss FYI using patch to build libavif AOMediaCodec/libavif@1556f21 allowed to pass tests)

#if AVIF_VERSION >= 90100
decoder->strictFlags &= ~AVIF_STRICT_PIXI_REQUIRED;
#endif

io = createAvifIOFromCtx(ctx);
if (!io) {
gd_error("avif error - Could not allocate memory");
Expand Down
3 changes: 0 additions & 3 deletions ext/gd/tests/imagecreatefromstring_avif.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ gd
if (!(imagetypes() & IMG_AVIF)) {
die('skip AVIF support required');
}
if (str_contains(PHP_OS, "FreeBSD")) {
die("xfail Currently failing on FreeBSD CI");
}
?>
--FILE--
<?php
Expand Down