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

2018-03-09 Thread Michael Niedermayer 
On Thu, Mar 08, 2018 at 02:48:19AM +0100, Jerome Martinez wrote:
> On 08/03/2018 01:17, Michael Niedermayer wrote:
> >
> >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 modified, showing the scalar value.

>  ffv1dec.c |   39 +++
>  1 file changed, 31 insertions(+), 8 deletions(-)
> b91dc1d7d73ea648cb343b5c3ed8159dc9bcc015  
> 0001-avcodec-ffv1dec-add-missing-error-messages-when-a-fr.patch
> From 13db9fc4da1d0e531e516bd87d084233e231f0e7 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= 
> Date: Thu, 8 Mar 2018 02:40:21 +0100
> Subject: [PATCH] avcodec/ffv1dec: add missing error messages when a frame is
>  invalid
> 
> ---
>  libavcodec/ffv1dec.c | 39 +++
>  1 file changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> index 3d2ee2569f..06509dae60 100644
> --- a/libavcodec/ffv1dec.c
> +++ b/libavcodec/ffv1dec.c
> @@ -182,11 +182,22 @@ static int decode_slice_header(FFV1Context *f, 
> FFV1Context *fs)
>  fs->slice_y /= f->num_v_slices;
>  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)
> +if ((unsigned)fs->slice_width > f->width) {
> +av_log(f->avctx, AV_LOG_ERROR, "slice_width %d out of range\n", 
> (unsigned)fs->slice_width);
>  return -1;
> -if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width
> - || (unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height)
> +}
> +if ((unsigned)fs->slice_height > f->height) {
> +av_log(f->avctx, AV_LOG_ERROR, "slice_height %d out of range\n", 
> (unsigned)fs->slice_height);
> +return -1;
> +}
> +if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width) {
> +av_log(f->avctx, AV_LOG_ERROR, "slice_x+slice_width %lld out of 
> range\n", (unsigned)fs->slice_x + (uint64_t)fs->slice_width, 
> (unsigned)fs->slice_y);

%lld is incorrect for uint64_t
PRIu64 or PRId64 or something liek that is matching it

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-03-07 Thread Jerome Martinez 

On 08/03/2018 01:17, Michael Niedermayer wrote:


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 modified, showing the scalar value.
From 13db9fc4da1d0e531e516bd87d084233e231f0e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= 
Date: Thu, 8 Mar 2018 02:40:21 +0100
Subject: [PATCH] avcodec/ffv1dec: add missing error messages when a frame is
 invalid

---
 libavcodec/ffv1dec.c | 39 +++
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 3d2ee2569f..06509dae60 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -182,11 +182,22 @@ static int decode_slice_header(FFV1Context *f, 
FFV1Context *fs)
 fs->slice_y /= f->num_v_slices;
 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)
+if ((unsigned)fs->slice_width > f->width) {
+av_log(f->avctx, AV_LOG_ERROR, "slice_width %d out of range\n", 
(unsigned)fs->slice_width);
 return -1;
-if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width
- || (unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height)
+}
+if ((unsigned)fs->slice_height > f->height) {
+av_log(f->avctx, AV_LOG_ERROR, "slice_height %d out of range\n", 
(unsigned)fs->slice_height);
+return -1;
+}
+if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width) {
+av_log(f->avctx, AV_LOG_ERROR, "slice_x+slice_width %lld out of 
range\n", (unsigned)fs->slice_x + (uint64_t)fs->slice_width, 
(unsigned)fs->slice_y);
 return -1;
+}
+if ((unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height) {
+av_log(f->avctx, AV_LOG_ERROR, "slice_y+slice_height %lld out of 
range\n", (unsigned)fs->slice_y + (uint64_t)fs->slice_height);
+return -1;
+}
 
 for (i = 0; i < f->plane_count; i++) {
 PlaneContext * const p = &fs->plane[i];
@@ -432,8 +443,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 %i in global 
header\n", f->micro_version);
 return AVERROR_INVALIDDATA;
+}
 }
 f->ac = get_symbol(c, state, 0);
 
@@ -758,12 +771,22 @@ static int read_header(FFV1Context *f)
 fs->slice_y /= f->num_v_slices;
 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)
+if ((unsigned)fs->slice_width  > f->width) {
+av_log(f->avctx, AV_LOG_ERROR, "slice_width %d out of 
range\n", (unsigned)fs->slice_width);
 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)
+}
+if ((unsigned)fs->slice_height > f->height) {
+av_log(f->avctx, AV_LOG_ERROR, "slice_height %d out of 
range\n", (unsigned)fs->slice_height);
+return AVERROR_INVALIDDATA;
+}
+if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width) 
{
+av_log(f->avctx, AV_LOG_ERROR, "slice_x+slice_width %lld out 
of range\n", (unsigned)fs->slice_x + (uint64_t)fs->slice_width, 
(unsigned)fs->slice_y);
 return AVERROR_INVALIDDATA;
+}
+if ((unsigned)fs->slice_y + (uint64_t)fs->slice_height > 
f->height) {
+av_log(f->avctx, AV_LOG_ERROR, "slice_y+slice_height %lld out 
of range\n", (unsigned)fs->slice_y + (uint64_t)fs->slice_height);
+return AVERROR_INVALIDDATA;
+}
 }
 
 for (i = 0; i < f->plane_count; i++) {
-- 
2.13.3.windows.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-03-07 Thread Michael Niedermayer 
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?= 
> 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)

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-03-07 Thread Jerome Martinez 

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?= 
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");
 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++) {
-- 
2.13.3.windows.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel