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.

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

Add PNG support to the example apps #3

Closed
jrmuizel opened this issue Jul 9, 2019 · 18 comments
Closed

Add PNG support to the example apps #3

jrmuizel opened this issue Jul 9, 2019 · 18 comments

Comments

@jrmuizel
Copy link

jrmuizel commented Jul 9, 2019

This would be handy for testing

@joedrago joedrago added backlog Not being actively considered for work, undecided and removed backlog Not being actively considered for work, undecided labels Mar 5, 2020
@joedrago
Copy link
Collaborator

This is finally in, and will be in the next release.

(feel free to play with it in the master branch)

c647acc

@utack
Copy link

utack commented Mar 11, 2020

The arch aur package fails to build
/aur-avif-git/src/libavif/apps/shared/avifpng.c:17:17: error: variable ‘rowPointers’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
17 | png_bytep * rowPointers = NULL;

Who is to fix this, should the maintainer change the building process or can libavif even solve this?

@joedrago
Copy link
Collaborator

That's a weird one. I'm not a fan of libpng's choice to use setjmp(), but flow-wise, I think rowPointers is safe here. It is initialized to NULL before any longjmp is possible, and any call to longjmp eventually ends up at the cleanup label, which handles either state of rowPointers correctly.

Can you give me your compiler version? I'd like to repro the error here and see if I can come up with a proper solution in libavif.

@joedrago
Copy link
Collaborator

The Internet (tm) suggests I can flag anything that might get jumped over as volatile as a hint to the optimizer to not do odd things with it. That might be enough. I want to see it fix the warning myself though.

@joedrago
Copy link
Collaborator

Nevermind, I repro'd with the archlinux Docker image. I'll fix it.

@utack
Copy link

utack commented Mar 11, 2020

Sorry you were too fast for me to respond on this one
Thank you for looking into it

@joedrago
Copy link
Collaborator

I think this will fix it, give it a shot.

79e4c6c

@utack
Copy link

utack commented Mar 11, 2020

It did indeed fix it!
Looking forward to trying png input

@utack
Copy link

utack commented Mar 11, 2020

Sorry to add something else to this

(gdb) backtrace
#0 0x00007ffff7530210 in ?? ()
#1 0x00007ffff7f14461 in ?? () from /usr/lib/libpng16.so.16
#2 0x00007ffff7f1fc48 in ?? () from /usr/lib/libpng16.so.16
#3 0x00007ffff7f293bf in ?? () from /usr/lib/libpng16.so.16
#4 0x00007ffff7f1cd79 in png_read_image () from /usr/lib/libpng16.so.16
#5 0x000055555555758c in ?? ()
#6 0x0000555555556488 in ?? ()
#7 0x00007ffff7d49023 in __libc_start_main () from /usr/lib/libc.so.6
#8 0x0000555555556e5e in ?? ()

I might look into this myself when I have time, but seems like something in libpng is not quite sorted correctly

@joedrago
Copy link
Collaborator

Yuck, it is my fault. avif->height isn't set yet. Fixing.

@joedrago
Copy link
Collaborator

Okay, I reverted that change and put a new one in.

ade6fe6

@utack
Copy link

utack commented Mar 11, 2020

Yes, now it works, thank you!

@joedrago
Copy link
Collaborator

As a note, I just fixed 16bpc PNG writing, my call to png_set_swap() was being ignored. I'm going to push a new version/tag in a few minutes.

@utack
Copy link

utack commented Mar 11, 2020

Something still seems odd about reading/writing png

Take this file for example
in

avifenc --max 0 in.png out.avif
avifdec out.avif reconstructed.png

identify -verbose says amongst other differences

Blue:
  min: 37  (0.145098)
  max: 120 (0.470588)
  mean: 80.5176 (0.315755)

Blue:
  min: 36  (0.141176)
  max: 121 (0.47451)
  mean: 80.6096 (0.316116)

@joedrago
Copy link
Collaborator

When converting RGB->YUV->RGB (even 444), you can have a drift of up to one codepoint in a channel. If I run

avifenc -c aom --min 0 --max 0 -s 0 blue.png blue.avif

I see a max drift of 1, which is expected.

image

@utack
Copy link

utack commented Mar 11, 2020

Understood, thanks for the clarification

@joedrago
Copy link
Collaborator

@utack I believe I may have done you a bit of a disservice by not being clearer in my response. I've recently had a discussion here which started from my quote above:

https://old.reddit.com/r/AV1/comments/g632tn/is_avif_from_the_program_xnview_really_lossless/forfmsj/?context=8

That reply should not only give you much more context about what I meant here, but also some hints as to avoid this issue in the future (with a more recent libavif / avifenc). Cheers!

@utack
Copy link

utack commented Apr 27, 2020

Thank you @joedrago I have followed it!
I am not working with avif in a professional capacity so I was satisfied with your previous response, but understanding more of the technical details has certainly taught me more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants