[PATCH 05/21] Change eval internal functions and ff_parse() interface.

Stefano Sabatini stefano.sabatini-lala
Tue Apr 6 00:13:04 CEST 2010


Make them return an error code and print the error on the log rather
than set a constant error string in the Parser.

Allow ff_parse() to provide more information about the reason of the
failure, and allow the error message to be more expressive, as it is
not anymore a generic const char * string.
---
 libavcodec/eval.c        |  169 ++++++++++++++++++++++++++++++----------------
 libavcodec/eval.h        |   12 ++--
 libavcodec/ratecontrol.c |   11 ++--
 3 files changed, 122 insertions(+), 70 deletions(-)

diff --git a/libavcodec/eval.c b/libavcodec/eval.c
index 182bbc5..72997b4 100644
--- a/libavcodec/eval.c
+++ b/libavcodec/eval.c
@@ -47,7 +47,6 @@ typedef struct Parser{
     double (**func2)(void *, double a, double b); // NULL terminated
     const char **func2_name;          // NULL terminated
     void *opaque;
-    const char **error;
 #define VARS 10
     double var[VARS];
 } Parser;
@@ -171,7 +170,7 @@ static double eval_expr(Parser * p, AVExpr * e) {
     return NAN;
 }
 
-static AVExpr * parse_expr(Parser *p);
+static int parse_expr(AVExpr **expr, Parser *p, void *log_ctx);
 
 void ff_free_expr(AVExpr * e) {
     if (!e) return;
@@ -180,20 +179,21 @@ void ff_free_expr(AVExpr * e) {
     av_freep(&e);
 }
 
-static AVExpr * parse_primary(Parser *p) {
+static int parse_primary(AVExpr **expr, Parser *p, void *log_ctx) {
     AVExpr * d = av_mallocz(sizeof(AVExpr));
     char *next= p->s;
-    int i;
+    int i, ret;
 
     if (!d)
-        return NULL;
+        return AVERROR(ENOMEM);
 
     /* number */
     d->value = av_strtod(p->s, &next);
     if(next != p->s){
         d->type = e_value;
         p->s= next;
-        return d;
+        *expr = d;
+        return 0;
     }
     d->value = 1;
 
@@ -203,38 +203,47 @@ static AVExpr * parse_primary(Parser *p) {
             p->s+= strlen(p->const_name[i]);
             d->type = e_const;
             d->a.const_index = i;
-            return d;
+            *expr = d;
+            return 0;
         }
     }
 
     p->s= strchr(p->s, '(');
     if(p->s==NULL){
-        *p->error = "undefined constant or missing (";
+        av_log(log_ctx, AV_LOG_ERROR, "undefined constant or missing (\n");
         p->s= next;
         ff_free_expr(d);
-        return NULL;
+        return AVERROR(EINVAL);
     }
     p->s++; // "("
     if (*next == '(') { // special case do-nothing
         av_freep(&d);
-        d = parse_expr(p);
+        if ((ret = parse_expr(&d, p, log_ctx)) < 0)
+            return ret;
         if(p->s[0] != ')'){
-            *p->error = "missing )";
+            av_log(log_ctx, AV_LOG_ERROR, "missing )\n");
             ff_free_expr(d);
-            return NULL;
+            return AVERROR(EINVAL);
         }
         p->s++; // ")"
-        return d;
+        *expr = d;
+        return 0;
+    }
+    if ((ret = parse_expr(&d->param[0], p, log_ctx)) < 0) {
+       ff_free_expr(d);
+       return ret;
     }
-    d->param[0] = parse_expr(p);
     if(p->s[0]== ','){
         p->s++; // ","
-        d->param[1] = parse_expr(p);
+        if ((ret = parse_expr(&d->param[1], p, log_ctx)) < 0) {
+            ff_free_expr(d);
+            return ret;
+        }
     }
     if(p->s[0] != ')'){
-        *p->error = "missing )";
+        av_log(log_ctx, AV_LOG_ERROR, "missing )\n");
         ff_free_expr(d);
-        return NULL;
+        return AVERROR(EINVAL);
     }
     p->s++; // ")"
 
@@ -269,7 +278,8 @@ static AVExpr * parse_primary(Parser *p) {
             if(strmatch(next, p->func1_name[i])){
                 d->a.func1 = p->func1[i];
                 d->type = e_func1;
-                return d;
+                *expr = d;
+                return 0;
             }
         }
 
@@ -277,16 +287,18 @@ static AVExpr * parse_primary(Parser *p) {
             if(strmatch(next, p->func2_name[i])){
                 d->a.func2 = p->func2[i];
                 d->type = e_func2;
-                return d;
+                *expr = d;
+                return 0;
             }
         }
 
-        *p->error = "unknown function";
+        av_log(log_ctx, AV_LOG_ERROR, "unknown function\n");
         ff_free_expr(d);
-        return NULL;
+        return AVERROR(EINVAL);
     }
 
-    return d;
+    *expr = d;
+    return 0;
 }
 
 static AVExpr * new_eval_expr(int type, int value, AVExpr *p0, AVExpr *p1){
@@ -300,67 +312,102 @@ static AVExpr * new_eval_expr(int type, int value, AVExpr *p0, AVExpr *p1){
     return e;
 }
 
-static AVExpr * parse_pow(Parser *p, int *sign){
+static int parse_pow(AVExpr **expr, Parser *p, int *sign, void *log_ctx){
     *sign= (*p->s == '+') - (*p->s == '-');
     p->s += *sign&1;
-    return parse_primary(p);
+    return parse_primary(expr, p, log_ctx);
 }
 
-static AVExpr * parse_factor(Parser *p){
-    int sign, sign2;
-    AVExpr * e = parse_pow(p, &sign);
+static int parse_factor(AVExpr **expr, Parser *p, void *log_ctx){
+    int ret, sign, sign2;
+    AVExpr * e = NULL, * e1 = NULL;
+
+    if ((ret = parse_pow(&e, p, &sign, log_ctx)) < 0)
+        return ret;
     while(p->s[0]=='^'){
         p->s++;
-        e= new_eval_expr(e_pow, 1, e, parse_pow(p, &sign2));
+        if ((ret = parse_pow(&e1, p, &sign2, log_ctx)) < 0) {
+            ff_free_expr(e);
+            return ret;
+        }
+        e= new_eval_expr(e_pow, 1, e, e1);
         if (!e)
-            return NULL;
+            return AVERROR(ENOMEM);
         if (e->param[1]) e->param[1]->value *= (sign2|1);
     }
     if (e) e->value *= (sign|1);
-    return e;
+    *expr = e;
+    return 0;
 }
 
-static AVExpr * parse_term(Parser *p){
-    AVExpr * e = parse_factor(p);
+static int parse_term(AVExpr **expr, Parser *p, void *log_ctx){
+    AVExpr * e = NULL, * e1 = NULL;
+    int ret;
+
+    if ((ret = parse_factor(&e, p, log_ctx)) < 0)
+        return ret;
+
     while(p->s[0]=='*' || p->s[0]=='/'){
         int c= *p->s++;
-        e= new_eval_expr(c == '*' ? e_mul : e_div, 1, e, parse_factor(p));
+        if ((ret = parse_factor(&e1, p, log_ctx)) < 0) {
+            ff_free_expr(e);
+            return ret;
+        }
+        e= new_eval_expr(c == '*' ? e_mul : e_div, 1, e, e1);
         if (!e)
-            return NULL;
+            return AVERROR(ENOMEM);
     }
-    return e;
+    *expr = e;
+    return 0;
 }
 
-static AVExpr * parse_subexpr(Parser *p) {
-    AVExpr * e = parse_term(p);
+static int parse_subexpr(AVExpr **expr, Parser *p, void *log_ctx) {
+    AVExpr * e = NULL, * e1 = NULL;
+    int ret;
+
+    if ((ret = parse_term(&e, p, log_ctx)) < 0)
+        return ret;
+
     while(*p->s == '+' || *p->s == '-') {
-        e= new_eval_expr(e_add, 1, e, parse_term(p));
+        if ((ret = parse_term(&e1, p, log_ctx) < 0)) {
+            ff_free_expr(e);
+            return ret;
+        }
+        e= new_eval_expr(e_add, 1, e, e1);
         if (!e)
-            return NULL;
+            return AVERROR(ENOMEM);
     };
 
-    return e;
+    *expr = e;
+    return 0;
 }
 
-static AVExpr * parse_expr(Parser *p) {
-    AVExpr * e;
+static int parse_expr(AVExpr **expr, Parser *p, void *log_ctx) {
+    AVExpr * e = NULL, * e1 = NULL;
+    int ret;
 
     if(p->stack_index <= 0) //protect against stack overflows
-        return NULL;
+        return -1;
     p->stack_index--;
 
-    e = parse_subexpr(p);
+    if ((ret = parse_subexpr(&e, p, log_ctx)) < 0)
+        return ret;
 
     while(*p->s == ';') {
         p->s++;
-        e= new_eval_expr(e_last, 1, e, parse_subexpr(p));
+        if ((ret = parse_subexpr(&e1, p, log_ctx)) < 0) {
+            ff_free_expr(e);
+            return ret;
+        }
+        e= new_eval_expr(e_last, 1, e, e1);
         if (!e)
-            return NULL;
+            return AVERROR(ENOMEM);
     };
 
     p->stack_index++;
 
-    return e;
+    *expr = e;
+    return 0;
 }
 
 static int verify_expr(AVExpr * e) {
@@ -377,17 +424,17 @@ static int verify_expr(AVExpr * e) {
     }
 }
 
-AVExpr * ff_parse(const char *s, const char * const *const_name,
+int ff_parse(AVExpr **expr, const char *s, const char * const *const_name,
                double (**func1)(void *, double), const char **func1_name,
                double (**func2)(void *, double, double), const char **func2_name,
-               const char **error){
+               void *log_ctx){
     Parser p;
-    AVExpr *e = NULL;
     char *w = av_malloc(strlen(s) + 1);
     char *wp = w;
+    int ret;
 
     if (!w)
-        goto end;
+        return AVERROR(ENOMEM);
 
     while (*s)
         if (!isspace(*s++)) *wp++ = s[-1];
@@ -400,16 +447,16 @@ AVExpr * ff_parse(const char *s, const char * const *const_name,
     p.func1_name = func1_name;
     p.func2      = func2;
     p.func2_name = func2_name;
-    p.error= error;
 
-    e = parse_expr(&p);
-    if (!verify_expr(e)) {
-        ff_free_expr(e);
-        e = NULL;
+    ret = parse_expr(expr, &p, log_ctx);
+    if (ret < 0)
+        goto end;
+    if (!verify_expr(*expr)) {
+        ff_free_expr(*expr);
     }
 end:
     av_free(w);
-    return e;
+    return ret;
 }
 
 double ff_parse_eval(AVExpr * e, const double *const_value, void *opaque) {
@@ -424,9 +471,13 @@ double ff_eval2(const char *s, const double *const_value, const char * const *co
                double (**func1)(void *, double), const char **func1_name,
                double (**func2)(void *, double, double), const char **func2_name,
                void *opaque, const char **error){
-    AVExpr * e = ff_parse(s, const_name, func1, func1_name, func2, func2_name, error);
+    AVExpr * e;
     double d;
-    if (!e) return NAN;
+    int ret = ff_parse(&e, s, const_name, func1, func1_name, func2, func2_name, NULL);
+    if (ret < 0) {
+        *error = "Error occurred during parsing, check log";
+        return NAN;
+    }
     d = ff_parse_eval(e, const_value, opaque);
     ff_free_expr(e);
     return d;
diff --git a/libavcodec/eval.h b/libavcodec/eval.h
index c3c9942..be96a7d 100644
--- a/libavcodec/eval.h
+++ b/libavcodec/eval.h
@@ -51,20 +51,22 @@ double ff_eval2(const char *s, const double *const_value, const char * const *co
 
 /**
  * Parses a expression.
+ * @param expr in case of successfull parsing puts here an AVExpr
+ * which must be freed with ff_free_expr() by the user when it is not
+ * needed anymore
  * @param s expression as a zero terminated string for example "1+2^3+5*5+sin(2/3)"
  * @param func1 NULL terminated array of function pointers for functions which take 1 argument
  * @param func2 NULL terminated array of function pointers for functions which take 2 arguments
  * @param const_name NULL terminated array of zero terminated strings of constant identifers for example {"PI", "E", 0}
  * @param func1_name NULL terminated array of zero terminated strings of func1 identifers
  * @param func2_name NULL terminated array of zero terminated strings of func2 identifers
- * @param error pointer to a char* which is set to an error message if something goes wrong
- * @return AVExpr which must be freed with ff_free_expr() by the user when it is not needed anymore
- *         NULL if anything went wrong
+ * @return 0 in case of success, a negative error corresponding to an
+ * AVERROR code in case of failure
  */
-AVExpr * ff_parse(const char *s, const char * const *const_name,
+int ff_parse(AVExpr **expr, const char *s, const char * const *const_name,
                double (**func1)(void *, double), const char **func1_name,
                double (**func2)(void *, double, double), const char **func2_name,
-               const char **error);
+               void *log_ctx);
 /**
  * Evaluates a previously parsed expression.
  * @param const_value a zero terminated array of values for the identifers from ff_parse const_name
diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
index f1f9c06..dd05668 100644
--- a/libavcodec/ratecontrol.c
+++ b/libavcodec/ratecontrol.c
@@ -66,8 +66,7 @@ static inline double bits2qp(RateControlEntry *rce, double bits){
 int ff_rate_control_init(MpegEncContext *s)
 {
     RateControlContext *rcc= &s->rc_context;
-    int i;
-    const char *error = NULL;
+    int i, ret;
     static const char * const const_names[]={
         "PI",
         "E",
@@ -107,10 +106,10 @@ int ff_rate_control_init(MpegEncContext *s)
     };
     emms_c();
 
-    rcc->rc_eq_eval = ff_parse(s->avctx->rc_eq ? s->avctx->rc_eq : "tex^qComp", const_names, func1, func1_names, NULL, NULL, &error);
-    if (!rcc->rc_eq_eval) {
-        av_log(s->avctx, AV_LOG_ERROR, "Error parsing rc_eq \"%s\": %s\n", s->avctx->rc_eq, error? error : "");
-        return -1;
+    ret = ff_parse(&rcc->rc_eq_eval, s->avctx->rc_eq ? s->avctx->rc_eq : "tex^qComp", const_names, func1, func1_names, NULL, NULL, s->avctx);
+    if (ret < 0) {
+        av_log(s->avctx, AV_LOG_ERROR, "Error parsing rc_eq \"%s\"\n", s->avctx->rc_eq);
+        return ret;
     }
 
     for(i=0; i<5; i++){
-- 
1.7.0


--zYM0uCDKw75PZbzx--



More information about the ffmpeg-devel mailing list