PR #24367 opened by oj.vasquez830
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24367
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24367.patch
PNGs whose `eXIf` chunk does not parse are currently rejected outright:
`decode_frame_common()` returns the error from
`ff_decode_exif_attach_buffer()`, so a single bad metadata chunk discards an
otherwise perfectly decodable image.
This is inconsistent with the rest of the tree and with the spec:
* `eXIf` is an **ancillary** chunk. PNG (3rd edition), Ch. 13: *"it is
recommended that unexpected field values be treated as fatal errors only in
critical chunks. An unexpected value in an ancillary chunk can be handled by
ignoring the whole chunk as though it were an unknown chunk type."*
* libpng, the reference implementation, rejects a bad `eXIf` with
`png_chunk_benign_error()` and still decodes the image.
* `libavcodec/webp.c` and `libavcodec/mjpegdec.c` make the identical EXIF call
non-fatal — same warning string in webp's case.
* `decode_text_to_exif()` in this same file already treats a failed EXIF parse
as non-fatal for the `Raw profile type exif` tEXt variant.
The second hunk additionally tolerates a leading `"Exif\0\0"` marker. The chunk
is specified to start directly with the TIFF header, but it was designed by
copying a JPEG APP1 payload minus its header, and some encoders leak the 6-byte
marker through. ExifTool handles this explicitly (`Improper "Exif00" header in
EXIF chunk`), and both ImageMagick's and Pillow's PNG encoders carry a
defensive `startswith("Exif\0\0")` guard for the same reason. Skipping it lets
the metadata be recovered instead of thrown away.
### Reproduction
A 2x2 PNG whose `eXIf` payload is `"Exif\0\0"` + a valid little-endian TIFF
block.
Before (master, `818e5d965b`):
```
$ ffmpeg -i exif_exif00.png -c:v png -f md5 -
[png @ ...] invalid TIFF header in EXIF data: Invalid data found when
processing input
Output file is empty, nothing was encoded
Conversion failed!
```
After:
```
$ ffmpeg -i exif_exif00.png -c:v png -f md5 -
[png @ ...] ignoring superfluous Exif\0\0 prefix in eXIf chunk
Side data:
EXIF metadata: (37 bytes)
MD5=974ac04628edfc0359c9349039ab4d55
```
The MD5 and the EXIF side data are now identical to those of the equivalent
spec-conformant file, and re-encoding normalises the chunk back to a bare
`II*\0` TIFF header.
### Testing
* A spec-conformant `eXIf` PNG decodes byte-identically before and after the
patch (`974ac04628edfc0359c9349039ab4d55`), and its EXIF side data is unchanged
— no behaviour change for valid files.
* Malformed payloads (garbage, truncated TIFF magic, zero-length chunk) now
decode the image with a warning instead of failing.
* Encode/decode round-trip through `pngenc` verified: both variants re-emit a
conformant `II*\0` payload and re-decode identically.
* `tools/patcheck` reports nothing beyond the generic "Missing changelog entry".
Note there is currently no FATE coverage for the PNG `eXIf` chunk at all
(`tests/fate/exif.mak` covers TIFF, JPEG and WebP only). I'm happy to add a
test if a sample can be added to the FATE samples repo.
From bb58bea8190ecd2483b71887671d79776b27edec Mon Sep 17 00:00:00 2001
From: Oscar Arguello <[email protected]>
Date: Fri, 4 Sep 2026 09:54:45 -0700
Subject: [PATCH] avcodec/pngdec: do not fail decoding on malformed eXIf chunks
eXIf is an ancillary chunk, so per the PNG specification an unexpected
value in it should be handled by ignoring the chunk rather than by
rejecting the datastream. Currently any eXIf payload that fails to parse
aborts the whole frame, so a single bad metadata chunk throws away an
otherwise perfectly valid image.
Make that failure non-fatal, matching what webp.c and mjpegdec.c already
do for the very same av_exif_parse_buffer() call, and what
decode_text_to_exif() in this file already does for the "Raw profile
type exif" tEXt variant.
Additionally, tolerate a leading "Exif\0\0" marker. The chunk is
specified to start directly with the TIFF header, but it was designed by
copying a JPEG APP1 payload minus its header, and some encoders leak
that 6-byte marker through. libpng drops such a chunk as a benign error
and ExifTool skips the marker with a warning; skipping it here lets the
metadata be recovered rather than discarded.
Signed-off-by: Oscar Arguello <[email protected]>
Co-authored-by: Copilot <[email protected]>
---
libavcodec/pngdec.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 0030fa5615..3b21eeb296 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -746,16 +746,35 @@ static int decode_phys_chunk(AVCodecContext *avctx,
PNGDecContext *s,
static int decode_exif_chunk(AVCodecContext *avctx, PNGDecContext *s,
GetByteContext *gb)
{
+ int size;
+
if (!(s->hdr_state & PNG_IHDR)) {
av_log(avctx, AV_LOG_ERROR, "eXIf before IHDR\n");
return AVERROR_INVALIDDATA;
}
+ /*
+ * The chunk is specified to start with the TIFF header, but some encoders
+ * (e.g. ImageMagick) prepend the "Exif\0\0" marker that JPEG uses in APP1.
+ */
+ if (bytestream2_get_bytes_left(gb) >= 6 &&
+ !memcmp(gb->buffer, "Exif\0\0", 6)) {
+ av_log(avctx, AV_LOG_WARNING,
+ "ignoring superfluous Exif\\0\\0 prefix in eXIf chunk\n");
+ bytestream2_skipu(gb, 6);
+ }
+
+ size = bytestream2_get_bytes_left(gb);
+ if (size <= 0) {
+ av_log(avctx, AV_LOG_WARNING, "empty eXIf chunk\n");
+ return 0;
+ }
+
av_buffer_unref(&s->exif_data);
- s->exif_data = av_buffer_alloc(bytestream2_get_bytes_left(gb));
+ s->exif_data = av_buffer_alloc(size);
if (!s->exif_data)
return AVERROR(ENOMEM);
- bytestream2_get_buffer(gb, s->exif_data->data, s->exif_data->size);
+ bytestream2_get_bufferu(gb, s->exif_data->data, size);
return 0;
}
@@ -1743,10 +1762,9 @@ exit_loop:
FFSWAP(AVDictionary *, p->metadata, s->frame_metadata);
ret = ff_decode_exif_attach_buffer(avctx, p, &s->exif_data,
AV_EXIF_TIFF_HEADER);
FFSWAP(AVDictionary *, p->metadata, s->frame_metadata);
- if (ret < 0) {
+ // a broken ancillary chunk must not discard an otherwise valid image
+ if (ret < 0)
av_log(avctx, AV_LOG_WARNING, "unable to attach EXIF buffer\n");
- return ret;
- }
}
if (s->color_type == PNG_COLOR_TYPE_PALETTE && avctx->codec_id ==
AV_CODEC_ID_APNG) {
--
2.52.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
