[FFmpeg-devel] [PATCH] libavcodec/libx264.c: distinguish between x264 parameter errors

[Erik Slagter]

Hi all,

The current code in libx264.c (#define OPT_STR) does
not distinguish between two types of errors returned
by libx264: parameter not existing or parameter's value bad.

This ommitment has given me some headaches, because I
didn't understand why my "profile=high" was "bad". After
all it appeared that "profile" isn't a valid "x264opt" at
all and needs to specified on the command line on it's own.

To save others this frustration, I'd like to have this
trivial patch in, it makes ffmpeg throw different error
messages for both errors. I had to change the #define into
a proper function, but imho that's only for the better.

This is against current git.

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index cc5b983..2959ba1 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -198,14 +198,36 @@ static void check_default_settings(AVCodecContext *avctx)
       }
   }

-#define OPT_STR(opt, param)                                             \
-    do {                                                                \
-        if (param && x264_param_parse(&x4->params, opt, param) < 0) {   \
-            av_log(avctx, AV_LOG_ERROR,                                 \
-                   "bad value for '%s': '%s'\n", opt, param);           \
-            return -1;                                                  \
-        }                                                               \
-    } while (0);                                                        \
+static int opt_str(AVCodecContext *avctx, X264Context *x4, const char *opt, const char *param)
+{
+    switch(x264_param_parse(&x4->params, opt, param))
+    {
+        case(0):
+        {
+            break;
+        }
+
+        case(X264_PARAM_BAD_NAME):
+        {
+            av_log(avctx, AV_LOG_ERROR, "no such option: \"%s\"\n", opt);
+            return(0);
+        }
+
+        case(X264_PARAM_BAD_VALUE):
+        {
+            av_log(avctx, AV_LOG_ERROR, "bad value: \"%s\" for option \"%s\"\n", param, opt);
+            return(0);
+        }
+
+        default:
+        {
+            av_log(avctx, AV_LOG_ERROR, "unknown parameter error, option: \"%s\", value: \"%s\"\n", param, opt);
+            return(0);
+        }
+    }
+
+    return(1);
+}

   static av_cold int X264_init(AVCodecContext *avctx)
   {
@@ -308,7 +330,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
       x4->params.p_log_private        = avctx;
       x4->params.i_log_level          = X264_LOG_DEBUG;

-    OPT_STR("weightp", x4->weightp);
+    if(!opt_str(avctx, x4, "weightp", x4->weightp ? x4->weightp : "0"))
+        return(-1);

       x4->params.b_intra_refresh      = avctx->flags2 & CODEC_FLAG2_INTRA_REFRESH;
       x4->params.rc.i_bitrate         = avctx->bit_rate       / 1000;
@@ -328,7 +351,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
           }
       }

-    OPT_STR("stats", x4->stats);
+    if(!opt_str(avctx, x4, "stats", x4->stats ? x4->stats : "0"))
+        return(-1);

       // if neither crf nor cqp modes are selected we have to enable the RC
       // we do it this way because we cannot check if the bitrate has been set
@@ -341,14 +365,16 @@ static av_cold int X264_init(AVCodecContext *avctx)
               (float)avctx->rc_initial_buffer_occupancy / avctx->rc_buffer_size;
       }

-    OPT_STR("level", x4->level);
+    if(!opt_str(avctx, x4, "level", x4->level ? x4->level : "0"))
+        return(-1);

       if(x4->x264opts){
           const char *p= x4->x264opts;
           while(p){
               char param[256]={0}, val[256]={0};
               sscanf(p, "%255[^:=]=%255[^:]", param, val);
-            OPT_STR(param, val);
+            if(!opt_str(avctx, x4, param, val))
+                return(-1);
               p= strchr(p, ':');
               p+=!!p;
           }



-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5110 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110622/1f3d2282/attachment.p7s>