diff mbox series

[FFmpeg-devel] avcodec/mpeg12enc: replace return -1 with proper error codes

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

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang May 6, 2020, 11:20 p.m. UTC
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(-)

Comments

Nicolas George May 6, 2020, 11:21 p.m. UTC | #1
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,
Lance Wang May 7, 2020, 1:57 a.m. UTC | #2
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
Nicolas George May 7, 2020, 9:18 a.m. UTC | #3
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 mbox series

Patch

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