| Message ID | 1505729381.2298662.1489767561651@mail.yahoo.com |
|---|---|
| State | Superseded |
| Headers | show |
Hello, I tested your patch on mac, seems to work, and doesn't break exr fate test. Some comments : > --- > libavcodec/exr.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/libavcodec/exr.c b/libavcodec/exr.c > index 034920f..dec21af 100644 > --- a/libavcodec/exr.c > +++ b/libavcodec/exr.c > @@ -1726,6 +1726,25 @@ static int decode_frame(AVCodecContext *avctx, void > *data, > if (bytestream2_get_bytes_left(&s->gb) < nb_blocks * 8) > return AVERROR_INVALIDDATA; > > + // check line offset table and recreate if first entry is 0 > + if (bytestream2_peek_le64(&s->gb) == 0) > + { > You should probably test if the file is in scanline (using s->is_tile). After taking a quick look to the official library, this table reconstruction seems to be only for scanline picture. > + uint64_t *table = (uint64_t *)s->gb.buffer; > + uint8_t *head = avpkt->data; > + uint8_t *marker = head + bytestream2_tell(&s->gb) + nb_blocks * 8; > You should probably move variable declaration to the top of the function. > + > + av_log(s->avctx, AV_LOG_INFO, "Recreating invalid offset > table.\n"); > + > + // recreate the line offset table using the length field for each > line > + // decode_block() will check for out-of-bounds offsets, so do not > bother > + // to check here > + for (y = 0; y < nb_blocks; y++) > + { > + table[y] = marker - head; > + marker += ((uint32_t *)marker)[1] + 8; > + } > + } > + > > // save pointer we are going to use in decode_block > s->buf = avpkt->data; > s->buf_size = avpkt->size; > -- > You should also add a fate test for a short sample. Martin
Thanks Martin for your comments. I made the requested changes here.
--- libavcodec/exr.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/libavcodec/exr.c b/libavcodec/exr.cindex 034920f..265d44d 100644--- a/libavcodec/exr.c+++ b/libavcodec/exr.c@@ -1640,6 +1640,9 @@ static int decode_frame(AVCodecContext *avctx, void *data, int out_line_size; int nb_blocks;/* nb scanline or nb tile */
+ uint64_t *table;/* scanline offset table */+ uint8_t *marker;/* used to recreate invalid scanline offset table */+ bytestream2_init(&s->gb, avpkt->data, avpkt->size);
if ((ret = decode_header(s, picture)) < 0)@@ -1731,6 +1734,21 @@ static int decode_frame(AVCodecContext *avctx, void *data, s->buf_size = avpkt->size; ptr = picture->data[0];
+ // check offset table+ if (!s->is_tile && bytestream2_peek_le64(&s->gb) == 0)+ {+ table = (uint64_t *)s->gb.buffer;+ marker = avpkt->data + bytestream2_tell(&s->gb) + nb_blocks * 8;++ av_log(s->avctx, AV_LOG_DEBUG, "recreating invalid scanline offset table\n");++ for (y = 0; y < nb_blocks; y++)+ {+ table[y] = marker - s->buf;+ marker += ((uint32_t *)marker)[1] + 8;+ }+ }+ // Zero out the start if ymin is not 0 for (y = 0; y < s->ymin; y++) { memset(ptr, 0, out_line_size);--2.1.4
From: Martin Vignali <martin.vignali@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Sent: Saturday, March 18, 2017 7:14 AM
Subject: Re: [FFmpeg-devel] [PATCH] avcodec/exr detect invalid line offset table and recreate table
Hello,
I tested your patch on mac, seems to work, and doesn't break exr fate test.
Some comments :
> ---
> libavcodec/exr.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> index 034920f..dec21af 100644
> --- a/libavcodec/exr.c
> +++ b/libavcodec/exr.c
> @@ -1726,6 +1726,25 @@ static int decode_frame(AVCodecContext *avctx, void
> *data,
> if (bytestream2_get_bytes_left(&s->gb) < nb_blocks * 8)
> return AVERROR_INVALIDDATA;
>
> + // check line offset table and recreate if first entry is 0
> + if (bytestream2_peek_le64(&s->gb) == 0)
> + {
>
You should probably test if the file is in scanline (using s->is_tile).
After taking a quick look to the official library, this table
reconstruction seems to be only for scanline picture.
> + uint64_t *table = (uint64_t *)s->gb.buffer;
> + uint8_t *head = avpkt->data;
> + uint8_t *marker = head + bytestream2_tell(&s->gb) + nb_blocks * 8;
>
You should probably move variable declaration to the top of the function.
> +
> + av_log(s->avctx, AV_LOG_INFO, "Recreating invalid offset
> table.\n");
> +
> + // recreate the line offset table using the length field for each
> line
> + // decode_block() will check for out-of-bounds offsets, so do not
> bother
> + // to check here
> + for (y = 0; y < nb_blocks; y++)
> + {
> + table[y] = marker - head;
> + marker += ((uint32_t *)marker)[1] + 8;
> + }
> + }
> +
>
>
// save pointer we are going to use in decode_block
> s->buf = avpkt->data;
> s->buf_size = avpkt->size;
> --
>
You should also add a fate test for a short sample.
Martin
diff --git a/libavcodec/exr.c b/libavcodec/exr.c index 034920f..dec21af 100644 --- a/libavcodec/exr.c +++ b/libavcodec/exr.c @@ -1726,6 +1726,25 @@ static int decode_frame(AVCodecContext *avctx, void *data, if (bytestream2_get_bytes_left(&s->gb) < nb_blocks * 8) return AVERROR_INVALIDDATA; + // check line offset table and recreate if first entry is 0 + if (bytestream2_peek_le64(&s->gb) == 0) + { + uint64_t *table = (uint64_t *)s->gb.buffer; + uint8_t *head = avpkt->data; + uint8_t *marker = head + bytestream2_tell(&s->gb) + nb_blocks * 8; + + av_log(s->avctx, AV_LOG_INFO, "Recreating invalid offset table.\n"); + + // recreate the line offset table using the length field for each line + // decode_block() will check for out-of-bounds offsets, so do not bother + // to check here + for (y = 0; y < nb_blocks; y++) + { + table[y] = marker - head; + marker += ((uint32_t *)marker)[1] + 8; + } + } + // save pointer we are going to use in decode_block s->buf = avpkt->data; s->buf_size = avpkt->size;