[FFmpeg-devel] [PATCH] avcodec/exr detect invalid line offset table and recreate table

[Martin Vignali]

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