Message ID | 20170923010154.1663-2-michael@niedermayer.cc |
---|---|
State | New |
Headers | show |
On 9/22/2017 10:01 PM, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/snowenc.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c > index bb113d0a2b..f5497f958c 100644 > --- a/libavcodec/snowenc.c > +++ b/libavcodec/snowenc.c > @@ -50,7 +50,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > && (avctx->flags & AV_CODEC_FLAG_QSCALE) > && avctx->global_quality == 0){ > av_log(avctx, AV_LOG_ERROR, "The 9/7 wavelet is incompatible with lossless mode.\n"); > - return -1; > + return AVERROR(EINVAL); > } > #if FF_API_MOTION_EST > FF_DISABLE_DEPRECATION_WARNINGS > @@ -107,8 +107,8 @@ FF_ENABLE_DEPRECATION_WARNINGS > return AVERROR(ENOMEM); > } > if((avctx->flags&AV_CODEC_FLAG_PASS2) || !(avctx->flags&AV_CODEC_FLAG_QSCALE)){ > - if(ff_rate_control_init(&s->m) < 0) > - return -1; > + if((ret = ff_rate_control_init(&s->m)) < 0) > + return ret; ret = foo(); if (ret < 0) return ret; No more combined assignment and comparisons for new code if possible. It's too prone to mistakes. > } > s->pass1_rc= !(avctx->flags & (AV_CODEC_FLAG_QSCALE|AV_CODEC_FLAG_PASS2)); > > @@ -130,7 +130,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > break;*/ > default: > av_log(avctx, AV_LOG_ERROR, "pixel format not supported\n"); > - return -1; > + return AVERROR_PATCHWELCOME; > } > avcodec_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_h_shift, &s->chroma_v_shift); > > @@ -833,7 +833,7 @@ static int encode_subband_c0run(SnowContext *s, SubBand *b, const IDWTELEM *src, > for(y=0; y<h; y++){ > if(s->c.bytestream_end - s->c.bytestream < w*40){ > av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); > - return -1; > + return AVERROR(ENOMEM); > } > for(x=0; x<w; x++){ > int v, p=0; > Should be good aside from the above.
On Fri, Sep 22, 2017 at 10:14:01PM -0300, James Almer wrote: > On 9/22/2017 10:01 PM, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/snowenc.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c > > index bb113d0a2b..f5497f958c 100644 > > --- a/libavcodec/snowenc.c > > +++ b/libavcodec/snowenc.c > > @@ -50,7 +50,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > > && (avctx->flags & AV_CODEC_FLAG_QSCALE) > > && avctx->global_quality == 0){ > > av_log(avctx, AV_LOG_ERROR, "The 9/7 wavelet is incompatible with lossless mode.\n"); > > - return -1; > > + return AVERROR(EINVAL); > > } > > #if FF_API_MOTION_EST > > FF_DISABLE_DEPRECATION_WARNINGS > > @@ -107,8 +107,8 @@ FF_ENABLE_DEPRECATION_WARNINGS > > return AVERROR(ENOMEM); > > } > > if((avctx->flags&AV_CODEC_FLAG_PASS2) || !(avctx->flags&AV_CODEC_FLAG_QSCALE)){ > > - if(ff_rate_control_init(&s->m) < 0) > > - return -1; > > + if((ret = ff_rate_control_init(&s->m)) < 0) > > + return ret; > > ret = foo(); > if (ret < 0) > return ret; > > No more combined assignment and comparisons for new code if possible. > It's too prone to mistakes. I dont make that mistake (anymore), but yes it gives a bad example. Will change > > > } > > s->pass1_rc= !(avctx->flags & (AV_CODEC_FLAG_QSCALE|AV_CODEC_FLAG_PASS2)); > > > > @@ -130,7 +130,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > > break;*/ > > default: > > av_log(avctx, AV_LOG_ERROR, "pixel format not supported\n"); > > - return -1; > > + return AVERROR_PATCHWELCOME; > > } > > avcodec_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_h_shift, &s->chroma_v_shift); > > > > @@ -833,7 +833,7 @@ static int encode_subband_c0run(SnowContext *s, SubBand *b, const IDWTELEM *src, > > for(y=0; y<h; y++){ > > if(s->c.bytestream_end - s->c.bytestream < w*40){ > > av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); > > - return -1; > > + return AVERROR(ENOMEM); > > } > > for(x=0; x<w; x++){ > > int v, p=0; > > > > Should be good aside from the above. will apply thanks [...]
2017-09-23 3:14 GMT+02:00 James Almer <jamrial@gmail.com>: > On 9/22/2017 10:01 PM, Michael Niedermayer wrote: >> + if((ret = ff_rate_control_init(&s->m)) < 0) >> + return ret; > > ret = foo(); > if (ret < 0) > return ret; > > No more combined assignment and comparisons for > new code if possible. It's too prone to mistakes. This comment looks strange given today's patches from you. Carl Eugen
On 9/24/2017 8:06 PM, Carl Eugen Hoyos wrote: > 2017-09-23 3:14 GMT+02:00 James Almer <jamrial@gmail.com>: >> On 9/22/2017 10:01 PM, Michael Niedermayer wrote: > >>> + if((ret = ff_rate_control_init(&s->m)) < 0) >>> + return ret; >> >> ret = foo(); >> if (ret < 0) >> return ret; >> >> No more combined assignment and comparisons for >> new code if possible. It's too prone to mistakes. > > This comment looks strange given today's patches from you. > > Carl Eugen You mean the patch where i did a sed replace of a function name in code that existed beforehand? I don't think it's the same as manually adding a brand new case of assignment and comparison in one statement. Do you want me to split those lines before i push the patch? Guess it would kill two birds with one stone, so probably worth it.
2017-09-25 1:16 GMT+02:00 James Almer <jamrial@gmail.com>: > On 9/24/2017 8:06 PM, Carl Eugen Hoyos wrote: >> 2017-09-23 3:14 GMT+02:00 James Almer <jamrial@gmail.com>: >>> On 9/22/2017 10:01 PM, Michael Niedermayer wrote: >> >>>> + if((ret = ff_rate_control_init(&s->m)) < 0) >>>> + return ret; >>> >>> ret = foo(); >>> if (ret < 0) >>> return ret; >>> >>> No more combined assignment and comparisons for >>> new code if possible. It's too prone to mistakes. >> >> This comment looks strange given today's patches from you. > You mean the patch where i did a sed replace of a function > name in code that existed beforehand? Sorry, but I believe using sed here is worse than above code which is why I hadn't realized you used it. Please feel free to ignore, Carl Eugen
diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c index bb113d0a2b..f5497f958c 100644 --- a/libavcodec/snowenc.c +++ b/libavcodec/snowenc.c @@ -50,7 +50,7 @@ FF_ENABLE_DEPRECATION_WARNINGS && (avctx->flags & AV_CODEC_FLAG_QSCALE) && avctx->global_quality == 0){ av_log(avctx, AV_LOG_ERROR, "The 9/7 wavelet is incompatible with lossless mode.\n"); - return -1; + return AVERROR(EINVAL); } #if FF_API_MOTION_EST FF_DISABLE_DEPRECATION_WARNINGS @@ -107,8 +107,8 @@ FF_ENABLE_DEPRECATION_WARNINGS return AVERROR(ENOMEM); } if((avctx->flags&AV_CODEC_FLAG_PASS2) || !(avctx->flags&AV_CODEC_FLAG_QSCALE)){ - if(ff_rate_control_init(&s->m) < 0) - return -1; + if((ret = ff_rate_control_init(&s->m)) < 0) + return ret; } s->pass1_rc= !(avctx->flags & (AV_CODEC_FLAG_QSCALE|AV_CODEC_FLAG_PASS2)); @@ -130,7 +130,7 @@ FF_ENABLE_DEPRECATION_WARNINGS break;*/ default: av_log(avctx, AV_LOG_ERROR, "pixel format not supported\n"); - return -1; + return AVERROR_PATCHWELCOME; } avcodec_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_h_shift, &s->chroma_v_shift); @@ -833,7 +833,7 @@ static int encode_subband_c0run(SnowContext *s, SubBand *b, const IDWTELEM *src, for(y=0; y<h; y++){ if(s->c.bytestream_end - s->c.bytestream < w*40){ av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); - return -1; + return AVERROR(ENOMEM); } for(x=0; x<w; x++){ int v, p=0;
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/snowenc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)