[FFmpeg-devel,1/7] avcodec/ffv1dec: add missing error messages when a frame is invalid

Submitted by Jerome Martinez on March 7, 2018, 3:45 p.m.

Details

Message ID 909fd1a5-f76b-d981-37d7-f1998259c6d0@mediaarea.net
State New
Headers show

Commit Message

Jerome Martinez March 7, 2018, 3:45 p.m.
A buggy file (before the patch preventing such issue is applied):
ffmpeg -y -f lavfi -i mandelbrot=s=31x31 -vframes 1 -c ffv1 -slices 961 
a.mkv

Then with:
ffmpeg -y -f lavfi -i mandelbrot=s=31x31 -vframes 1 source.jpg
ffmpeg -y -i a.mkv a.jpg

There is no error message despite the fact the stream was not correctly 
decoded (a.jpg is not like source.jpg), but user is not informed that 
the decoding was not good.

This patch adds error message to the output when relevant.
From 04f7275bdefe56ca2ff5d175de6e392f60c31bc3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net>
Date: Wed, 7 Mar 2018 10:36:36 +0100
Subject: [PATCH 1/7] avcodec/ffv1dec: add missing error messages when a frame
 is invalid

---
 libavcodec/ffv1dec.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Michael Niedermayer March 8, 2018, 12:17 a.m.
On Wed, Mar 07, 2018 at 04:45:20PM +0100, Jerome Martinez wrote:
> A buggy file (before the patch preventing such issue is applied):
> ffmpeg -y -f lavfi -i mandelbrot=s=31x31 -vframes 1 -c ffv1 -slices 961
> a.mkv
> 
> Then with:
> ffmpeg -y -f lavfi -i mandelbrot=s=31x31 -vframes 1 source.jpg
> ffmpeg -y -i a.mkv a.jpg
> 
> There is no error message despite the fact the stream was not correctly
> decoded (a.jpg is not like source.jpg), but user is not informed that the
> decoding was not good.
> 
> This patch adds error message to the output when relevant.
> 
> 

>  ffv1dec.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 3c17506214ec584128ad4e194acee98737f987da  0001-avcodec-ffv1dec-add-missing-error-messages-when-a-fr.patch
> From 04f7275bdefe56ca2ff5d175de6e392f60c31bc3 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net>
> Date: Wed, 7 Mar 2018 10:36:36 +0100
> Subject: [PATCH 1/7] avcodec/ffv1dec: add missing error messages when a frame
>  is invalid
> 
> ---
>  libavcodec/ffv1dec.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> index 3d2ee2569f..94bd60ad2b 100644
> --- a/libavcodec/ffv1dec.c
> +++ b/libavcodec/ffv1dec.c
> @@ -296,6 +296,7 @@ static int decode_slice(AVCodecContext *c, void *arg)
>          if (decode_slice_header(f, fs) < 0) {
>              fs->slice_x = fs->slice_y = fs->slice_height = fs->slice_width = 0;
>              fs->slice_damaged = 1;
> +            av_log(f->avctx, AV_LOG_ERROR, "Invalid content found while parsing slice header\n");
>              return AVERROR_INVALIDDATA;
>          }
>      }

> @@ -432,8 +433,10 @@ static int read_extra_header(FFV1Context *f)
>      if (f->version > 2) {
>          c->bytestream_end -= 4;
>          f->micro_version = get_symbol(c, state, 0);
> -        if (f->micro_version < 0)
> +        if (f->micro_version < 0) {
> +            av_log(f->avctx, AV_LOG_ERROR, "Invalid micro_version in global header\n");

In the cases where the error is about a scalar value, that value should
be printed in the error message (unless it was alread printed elsewhere)

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 3d2ee2569f..94bd60ad2b 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -296,6 +296,7 @@  static int decode_slice(AVCodecContext *c, void *arg)
         if (decode_slice_header(f, fs) < 0) {
             fs->slice_x = fs->slice_y = fs->slice_height = fs->slice_width = 0;
             fs->slice_damaged = 1;
+            av_log(f->avctx, AV_LOG_ERROR, "Invalid content found while parsing slice header\n");
             return AVERROR_INVALIDDATA;
         }
     }
@@ -432,8 +433,10 @@  static int read_extra_header(FFV1Context *f)
     if (f->version > 2) {
         c->bytestream_end -= 4;
         f->micro_version = get_symbol(c, state, 0);
-        if (f->micro_version < 0)
+        if (f->micro_version < 0) {
+            av_log(f->avctx, AV_LOG_ERROR, "Invalid micro_version in global header\n");
             return AVERROR_INVALIDDATA;
+        }
     }
     f->ac = get_symbol(c, state, 0);
 
@@ -759,11 +762,15 @@  static int read_header(FFV1Context *f)
             fs->slice_width  = fs->slice_width  / f->num_h_slices - fs->slice_x;
             fs->slice_height = fs->slice_height / f->num_v_slices - fs->slice_y;
             if ((unsigned)fs->slice_width  > f->width ||
-                (unsigned)fs->slice_height > f->height)
+                (unsigned)fs->slice_height > f->height) {
+                av_log(f->avctx, AV_LOG_ERROR, "Invalid content found while parsing header\n");
                 return AVERROR_INVALIDDATA;
+            }
             if (   (unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width
-                || (unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height)
+                || (unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height) {
+                av_log(f->avctx, AV_LOG_ERROR, "Invalid content found while parsing header\n");
                 return AVERROR_INVALIDDATA;
+            }
         }
 
         for (i = 0; i < f->plane_count; i++) {