Aspect ratio defined by the PNG resolution chunk “pHYs” is misinterpreted as its reciprocal

[goodbye]

The PNG pHYs chunk describes the resolution/pixel density of an image, which can be in abstract units (in which case it only defines the pixel aspect ratio) or in pixels per metre.

FFmpeg interprets the X:Y resolution ratio directly as a X:Y PAR, but in actuality, it’s the reciprocal. This is because the resolution is specified as a “pixels per unit” value and not “units per pixel”, as is typically the case with PARs. The X:Y PAR is equal to the Y:X ratio of resolutions specified in the chunk.

Both of these images, from the libpng test suite, are supposed to have a DAR of 1:1, but FFmpeg misinterprets them to have DARs of 16:1 and 1:16:

• ​http://www.libpng.org/pub/png/PngSuite/cdhn2c08.png
• ​http://www.libpng.org/pub/png/PngSuite/cdfn2c08.png

The issue has been present from the very beginning, since parsing the pHYs chunk was introduced in commit 8288c2b6cb072b61f664bc8ab4c1b0a5a8db0ead, and is still present as of commit 1864025458021a2d2c542f56e268ee1106f84460 (libavcodec/pngdec.c lines 648–649)

The PNG encoder has the complementary problem of writing a reciprocal ratio into the pHYs chunk, introduced with commit f58f90238fc63090da34c9fdb1d06d724d929f6d (now libavcodec/pngenc.c lines 394–395)

[Balling]

As I understand Chrome shows it correctly, right? First link is horizontal image, and second is vertical.

[MasterQuestionable]

͏    Likely per the spec it doesn't... But unfixable at this point.
͏    (back-compatibility havoc)

[Balling]

Nah, Browsers render it differently compared to ffmpeg and mpv. So I think they render them correct.

[goodbye]

Replying to Balling:

As I understand Chrome shows it correctly, right? First link is horizontal image, and second is vertical.

No, it just ignores the chunk and renders the image with square pixels.

[Balling]

Right, ​https://issues.chromium.org/issues/40509465#comment19 I see

So the result image is just a square and indeed if you invert how you read the pHYs it will be square.

[MasterQuestionable]

͏    Differently broken in different implementations in different manner... 3D broken now.

[Balling]

Is the fix simply changing to

avctx->sample_aspect_ratio.den = bytestream2_get_be32(&s->gb);
+ avctx->sample_aspect_ratio.num = bytestream2_get_be32(&s->gb);

?

[goodbye]

Yes, it's sufficient to swap num and den. And probably swap them in the encoder too. And ensure the test suite covers this.