Re: [libav-devel] [PATCH 2/3] ffv1: propagate errors

2012-10-19 Thread Kostya Shishkov 
On Fri, Oct 19, 2012 at 09:55:41PM +0200, Luca Barbato wrote:
> ---
>  libavcodec/ffv1dec.c | 53 
> ++--
>  libavcodec/ffv1enc.c | 26 +-
>  2 files changed, 40 insertions(+), 39 deletions(-)

LGTM
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 2/3] ffv1: propagate errors

2012-10-19 Thread Luca Barbato 
---
 libavcodec/ffv1dec.c | 53 ++--
 libavcodec/ffv1enc.c | 26 +-
 2 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 6877b78..f8c232f 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -291,7 +291,7 @@ static int read_quant_table(RangeCoder *c, int16_t 
*quant_table, int scale)
 int len = get_symbol(c, state, 0) + 1;
 
 if (len + i > 128)
-return -1;
+return AVERROR_INVALIDDATA;
 
 while (len--) {
 quant_table[i] = scale * v;
@@ -315,7 +315,7 @@ static int read_quant_tables(RangeCoder *c,
 for (i = 0; i < 5; i++) {
 context_count *= read_quant_table(c, quant_table[i], context_count);
 if (context_count > 32768U) {
-return -1;
+return AVERROR_INVALIDDATA;
 }
 }
 return (context_count + 1) / 2;
@@ -352,19 +352,19 @@ static int read_extra_header(FFV1Context *f)
 if (f->num_h_slices > (unsigned)f->width ||
 f->num_v_slices > (unsigned)f->height) {
 av_log(f->avctx, AV_LOG_ERROR, "too many slices\n");
-return -1;
+return AVERROR_INVALIDDATA;
 }
 
 f->quant_table_count = get_symbol(c, state, 0);
 
 if (f->quant_table_count > (unsigned)MAX_QUANT_TABLES)
-return -1;
+return AVERROR_INVALIDDATA;
 
 for (i = 0; i < f->quant_table_count; i++) {
 f->context_count[i] = read_quant_tables(c, f->quant_tables[i]);
 if (f->context_count[i] < 0) {
 av_log(f->avctx, AV_LOG_ERROR, "read_quant_table error\n");
-return -1;
+return f->context_count[i];
 }
 }
 
@@ -426,7 +426,7 @@ static int read_header(FFV1Context *f)
 break;
 default:
 av_log(f->avctx, AV_LOG_ERROR, "format not supported\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 } else {
 switch (16 * f->chroma_h_shift + f->chroma_v_shift) {
@@ -441,19 +441,19 @@ static int read_header(FFV1Context *f)
 break;
 default:
 av_log(f->avctx, AV_LOG_ERROR, "format not supported\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 }
 } else if (f->colorspace == 1) {
 if (f->chroma_h_shift || f->chroma_v_shift) {
 av_log(f->avctx, AV_LOG_ERROR,
"chroma subsampling not supported in this colorspace\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 f->avctx->pix_fmt = AV_PIX_FMT_RGB32;
 } else {
 av_log(f->avctx, AV_LOG_ERROR, "colorspace not supported\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 
 av_dlog(f->avctx, "%d %d %d\n",
@@ -463,12 +463,12 @@ static int read_header(FFV1Context *f)
 context_count = read_quant_tables(c, f->quant_table);
 if (context_count < 0) {
 av_log(f->avctx, AV_LOG_ERROR, "read_quant_table error\n");
-return -1;
+return context_count;
 }
 } else {
 f->slice_count = get_symbol(c, state, 0);
 if (f->slice_count > (unsigned)MAX_SLICES)
-return -1;
+return AVERROR_INVALIDDATA;
 }
 
 for (j = 0; j < f->slice_count; j++) {
@@ -487,10 +487,10 @@ static int read_header(FFV1Context *f)
 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)
-return -1;
+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)
-return -1;
+return AVERROR_INVALIDDATA;
 }
 
 for (i = 0; i < f->plane_count; i++) {
@@ -501,7 +501,7 @@ static int read_header(FFV1Context *f)
 if (idx > (unsigned)f->quant_table_count) {
 av_log(f->avctx, AV_LOG_ERROR,
"quant_table_index out of range\n");
-return -1;
+return AVERROR_INVALIDDATA;
 }
 p->quant_table_index = idx;
 memcpy(p->quant_table, f->quant_tables[idx],
@@ -525,14 +525,15 @@ static int read_header(FFV1Context *f)
 static av_cold int ffv1_decode_init(AVCodecContext *avctx)
 {
 FFV1Context *f = avctx->priv_data;
+int ret;
 
 ffv1_common_init(avctx);
 
-if (avctx->extradata && read_extra_header(f) < 0)
-return -1;
+if (avctx->extradata && (ret = read_extra_header(f)) < 0)
+return ret;
 
-if (ffv1_init_slice_contexts(f) < 0)
-return -1;
+if ((ret = ffv1_init_slice_contexts(f)) < 0)

Re: [libav-devel] [PATCH 2/3] ffv1: propagate errors

2012-10-19 Thread Kostya Shishkov 
On Fri, Oct 19, 2012 at 12:16:34PM +0200, Luca Barbato wrote:
> ---
>  libavcodec/ffv1dec.c | 53 
> ++--
>  libavcodec/ffv1enc.c | 26 +-
>  2 files changed, 40 insertions(+), 39 deletions(-)
> 
> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> index 6877b78..c05c3a0 100644
> --- a/libavcodec/ffv1dec.c
> +++ b/libavcodec/ffv1dec.c
> @@ -441,19 +441,19 @@ static int read_header(FFV1Context *f)
>  break;
>  default:
>  av_log(f->avctx, AV_LOG_ERROR, "format not supported\n");
> -return -1;
> +return AVERROR(ENOSYS);
>  }
>  }
>  } else if (f->colorspace == 1) {
>  if (f->chroma_h_shift || f->chroma_v_shift) {
>  av_log(f->avctx, AV_LOG_ERROR,
> "chroma subsampling not supported in this colorspace\n");
> -return -1;
> +return AVERROR(ENOSYS);
>  }
>  f->avctx->pix_fmt = AV_PIX_FMT_RGB32;
>  } else {
>  av_log(f->avctx, AV_LOG_ERROR, "colorspace not supported\n");
> -return -1;
> +return AVERROR(ENOSYS);
>  }
>  
>  av_dlog(f->avctx, "%d %d %d\n",

might be a bit bikesheddy but this code is fine with me

> @@ -561,10 +562,10 @@ static int ffv1_decode_frame(AVCodecContext *avctx, 
> void *data,
>  p->pict_type = AV_PICTURE_TYPE_I; // FIXME: I vs. P
>  if (get_rac(c, &keystate)) {
>  p->key_frame = 1;
> -if (read_header(f) < 0)
> -return -1;
> -if (ffv1_init_slice_state(f) < 0)
> -return -1;
> +if ((ret = read_header(f)) < 0)
> +return ret;
> +if ((ret =ffv1_init_slice_state(f)) < 0)

diego-ahem

> +return ret;
>  
>  ffv1_clear_state(f);
>  } else {
[...]

in general LGTM
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 2/3] ffv1: propagate errors

2012-10-19 Thread Luca Barbato 
---
 libavcodec/ffv1dec.c | 53 ++--
 libavcodec/ffv1enc.c | 26 +-
 2 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 6877b78..c05c3a0 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -291,7 +291,7 @@ static int read_quant_table(RangeCoder *c, int16_t 
*quant_table, int scale)
 int len = get_symbol(c, state, 0) + 1;
 
 if (len + i > 128)
-return -1;
+return AVERROR_INVALIDDATA;
 
 while (len--) {
 quant_table[i] = scale * v;
@@ -315,7 +315,7 @@ static int read_quant_tables(RangeCoder *c,
 for (i = 0; i < 5; i++) {
 context_count *= read_quant_table(c, quant_table[i], context_count);
 if (context_count > 32768U) {
-return -1;
+return AVERROR_INVALIDDATA;
 }
 }
 return (context_count + 1) / 2;
@@ -352,19 +352,19 @@ static int read_extra_header(FFV1Context *f)
 if (f->num_h_slices > (unsigned)f->width ||
 f->num_v_slices > (unsigned)f->height) {
 av_log(f->avctx, AV_LOG_ERROR, "too many slices\n");
-return -1;
+return AVERROR_INVALIDDATA;
 }
 
 f->quant_table_count = get_symbol(c, state, 0);
 
 if (f->quant_table_count > (unsigned)MAX_QUANT_TABLES)
-return -1;
+return AVERROR_INVALIDDATA;
 
 for (i = 0; i < f->quant_table_count; i++) {
 f->context_count[i] = read_quant_tables(c, f->quant_tables[i]);
 if (f->context_count[i] < 0) {
 av_log(f->avctx, AV_LOG_ERROR, "read_quant_table error\n");
-return -1;
+return f->context_count[i];
 }
 }
 
@@ -426,7 +426,7 @@ static int read_header(FFV1Context *f)
 break;
 default:
 av_log(f->avctx, AV_LOG_ERROR, "format not supported\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 } else {
 switch (16 * f->chroma_h_shift + f->chroma_v_shift) {
@@ -441,19 +441,19 @@ static int read_header(FFV1Context *f)
 break;
 default:
 av_log(f->avctx, AV_LOG_ERROR, "format not supported\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 }
 } else if (f->colorspace == 1) {
 if (f->chroma_h_shift || f->chroma_v_shift) {
 av_log(f->avctx, AV_LOG_ERROR,
"chroma subsampling not supported in this colorspace\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 f->avctx->pix_fmt = AV_PIX_FMT_RGB32;
 } else {
 av_log(f->avctx, AV_LOG_ERROR, "colorspace not supported\n");
-return -1;
+return AVERROR(ENOSYS);
 }
 
 av_dlog(f->avctx, "%d %d %d\n",
@@ -463,12 +463,12 @@ static int read_header(FFV1Context *f)
 context_count = read_quant_tables(c, f->quant_table);
 if (context_count < 0) {
 av_log(f->avctx, AV_LOG_ERROR, "read_quant_table error\n");
-return -1;
+return context_count;
 }
 } else {
 f->slice_count = get_symbol(c, state, 0);
 if (f->slice_count > (unsigned)MAX_SLICES)
-return -1;
+return AVERROR_INVALIDDATA;
 }
 
 for (j = 0; j < f->slice_count; j++) {
@@ -487,10 +487,10 @@ static int read_header(FFV1Context *f)
 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)
-return -1;
+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)
-return -1;
+return AVERROR_INVALIDDATA;
 }
 
 for (i = 0; i < f->plane_count; i++) {
@@ -501,7 +501,7 @@ static int read_header(FFV1Context *f)
 if (idx > (unsigned)f->quant_table_count) {
 av_log(f->avctx, AV_LOG_ERROR,
"quant_table_index out of range\n");
-return -1;
+return AVERROR_INVALIDDATA;
 }
 p->quant_table_index = idx;
 memcpy(p->quant_table, f->quant_tables[idx],
@@ -525,14 +525,15 @@ static int read_header(FFV1Context *f)
 static av_cold int ffv1_decode_init(AVCodecContext *avctx)
 {
 FFV1Context *f = avctx->priv_data;
+int ret;
 
 ffv1_common_init(avctx);
 
-if (avctx->extradata && read_extra_header(f) < 0)
-return -1;
+if (avctx->extradata && (ret = read_extra_header(f)) < 0)
+return ret;
 
-if (ffv1_init_slice_contexts(f) < 0)
-return -1;
+if ((ret = ffv1_init_slice_contexts(f)) < 0)