Message ID | 1588807211-15105-1-git-send-email-lance.lmwang@gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] avcodec/mpeg12enc: replace return -1 with proper error codes | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
lance.lmwang@gmail.com (12020-05-07): > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavcodec/mpeg12enc.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c > index 643ba81..f317c63 100644 > --- a/libavcodec/mpeg12enc.c > +++ b/libavcodec/mpeg12enc.c > @@ -142,13 +142,13 @@ static av_cold int encode_init(AVCodecContext *avctx) > MpegEncContext *s = avctx->priv_data; > > if (ff_mpv_encode_init(avctx) < 0) ^^^^ > - return -1; > + return AVERROR(EINVAL); You have an accurate return code available. > > if (find_frame_rate_index(s) < 0) { > if (s->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) { > av_log(avctx, AV_LOG_ERROR, "MPEG-1/2 does not support %d/%d fps\n", > avctx->time_base.den, avctx->time_base.num); > - return -1; > + return AVERROR(EINVAL); > } else { > av_log(avctx, AV_LOG_INFO, > "MPEG-1/2 does not support %d/%d fps, there may be AV sync issues\n", > @@ -159,7 +159,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > if (avctx->profile == FF_PROFILE_UNKNOWN) { > if (avctx->level != FF_LEVEL_UNKNOWN) { > av_log(avctx, AV_LOG_ERROR, "Set profile and level\n"); > - return -1; > + return AVERROR(EINVAL); > } > /* Main or 4:2:2 */ > avctx->profile = s->chroma_format == CHROMA_420 ? FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422; > @@ -175,7 +175,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > if (avctx->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != CHROMA_420) { > av_log(avctx, AV_LOG_ERROR, > "Only High(1) and 4:2:2(0) profiles support 4:2:2 color sampling\n"); > - return -1; > + return AVERROR(EINVAL); > } > if (avctx->width <= 720 && avctx->height <= 576) > avctx->level = 8; /* Main */ > @@ -205,7 +205,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > if (s->drop_frame_timecode && s->frame_rate_index != 4) { > av_log(avctx, AV_LOG_ERROR, > "Drop frame time code only allowed with 1001/30000 fps\n"); > - return -1; > + return AVERROR(EINVAL); > } > > #if FF_API_PRIVATE_OPT Regards,
On Thu, May 07, 2020 at 01:21:44AM +0200, Nicolas George wrote: > lance.lmwang@gmail.com (12020-05-07): > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavcodec/mpeg12enc.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c > > index 643ba81..f317c63 100644 > > --- a/libavcodec/mpeg12enc.c > > +++ b/libavcodec/mpeg12enc.c > > @@ -142,13 +142,13 @@ static av_cold int encode_init(AVCodecContext *avctx) > > MpegEncContext *s = avctx->priv_data; > > > > > if (ff_mpv_encode_init(avctx) < 0) > ^^^^ > > - return -1; > > + return AVERROR(EINVAL); > > You have an accurate return code available. Yeah, then I had to change the ff_mpv_encode_init() return with accurate error code also. > > > > > if (find_frame_rate_index(s) < 0) { > > if (s->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) { > > av_log(avctx, AV_LOG_ERROR, "MPEG-1/2 does not support %d/%d fps\n", > > avctx->time_base.den, avctx->time_base.num); > > - return -1; > > + return AVERROR(EINVAL); > > } else { > > av_log(avctx, AV_LOG_INFO, > > "MPEG-1/2 does not support %d/%d fps, there may be AV sync issues\n", > > @@ -159,7 +159,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > > if (avctx->profile == FF_PROFILE_UNKNOWN) { > > if (avctx->level != FF_LEVEL_UNKNOWN) { > > av_log(avctx, AV_LOG_ERROR, "Set profile and level\n"); > > - return -1; > > + return AVERROR(EINVAL); > > } > > /* Main or 4:2:2 */ > > avctx->profile = s->chroma_format == CHROMA_420 ? FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422; > > @@ -175,7 +175,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > > if (avctx->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != CHROMA_420) { > > av_log(avctx, AV_LOG_ERROR, > > "Only High(1) and 4:2:2(0) profiles support 4:2:2 color sampling\n"); > > - return -1; > > + return AVERROR(EINVAL); > > } > > if (avctx->width <= 720 && avctx->height <= 576) > > avctx->level = 8; /* Main */ > > @@ -205,7 +205,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > > if (s->drop_frame_timecode && s->frame_rate_index != 4) { > > av_log(avctx, AV_LOG_ERROR, > > "Drop frame time code only allowed with 1001/30000 fps\n"); > > - return -1; > > + return AVERROR(EINVAL); > > } > > > > #if FF_API_PRIVATE_OPT > > Regards, > > -- > Nicolas George
lance.lmwang@gmail.com (12020-05-07):
> Yeah, then I had to change the ff_mpv_encode_init() return with accurate error code also.
If necessary yes. But it can be done later if you have other priorities.
But I consider better to correctly forward an invalid error code than to
incorrectly invent a new error code. One would be fixed later, the other
not.
Regards,
diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c index 643ba81..f317c63 100644 --- a/libavcodec/mpeg12enc.c +++ b/libavcodec/mpeg12enc.c @@ -142,13 +142,13 @@ static av_cold int encode_init(AVCodecContext *avctx) MpegEncContext *s = avctx->priv_data; if (ff_mpv_encode_init(avctx) < 0) - return -1; + return AVERROR(EINVAL); if (find_frame_rate_index(s) < 0) { if (s->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) { av_log(avctx, AV_LOG_ERROR, "MPEG-1/2 does not support %d/%d fps\n", avctx->time_base.den, avctx->time_base.num); - return -1; + return AVERROR(EINVAL); } else { av_log(avctx, AV_LOG_INFO, "MPEG-1/2 does not support %d/%d fps, there may be AV sync issues\n", @@ -159,7 +159,7 @@ static av_cold int encode_init(AVCodecContext *avctx) if (avctx->profile == FF_PROFILE_UNKNOWN) { if (avctx->level != FF_LEVEL_UNKNOWN) { av_log(avctx, AV_LOG_ERROR, "Set profile and level\n"); - return -1; + return AVERROR(EINVAL); } /* Main or 4:2:2 */ avctx->profile = s->chroma_format == CHROMA_420 ? FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422; @@ -175,7 +175,7 @@ static av_cold int encode_init(AVCodecContext *avctx) if (avctx->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != CHROMA_420) { av_log(avctx, AV_LOG_ERROR, "Only High(1) and 4:2:2(0) profiles support 4:2:2 color sampling\n"); - return -1; + return AVERROR(EINVAL); } if (avctx->width <= 720 && avctx->height <= 576) avctx->level = 8; /* Main */ @@ -205,7 +205,7 @@ static av_cold int encode_init(AVCodecContext *avctx) if (s->drop_frame_timecode && s->frame_rate_index != 4) { av_log(avctx, AV_LOG_ERROR, "Drop frame time code only allowed with 1001/30000 fps\n"); - return -1; + return AVERROR(EINVAL); } #if FF_API_PRIVATE_OPT