[x265] [PATCH] add fanout validation module to check param compatibility

[Steve Borho]

On 09/15, sagar at multicorewareinc.com wrote:
> # HG changeset patch
> # User Sagar Kotecha <sagar at multicorewareinc.com>
> # Date 1410784964 -19800
> #      Mon Sep 15 18:12:44 2014 +0530
> # Node ID 48ce21d7a0633a2650994d58987c5489608b6c57
> # Parent  67ee212bbf78f7192e5c5af6b53304468bfa55b1
> add fanout validation module to check param compatibility
> 
> diff -r 67ee212bbf78 -r 48ce21d7a063 source/x265.cpp
> --- a/source/x265.cpp	Mon Sep 15 16:09:52 2014 +0530
> +++ b/source/x265.cpp	Mon Sep 15 18:12:44 2014 +0530
> @@ -262,6 +262,7 @@
>      bool parseQPFile(x265_picture &pic_org);
>      void readAnalysisFile(x265_picture* pic, x265_param*);
>      void writeAnalysisFile(x265_picture* pic, x265_param*);
> +    bool validateFanout(x265_param*);
>  };
>  
>  void CLIOptions::destroy()
> @@ -755,6 +756,91 @@
>      return false;
>  }
>  
> +bool CLIOptions::validateFanout(x265_param *param)
> +{
> +#define CMP_OPT_FANOUT(opt, param_val)\
> +    {\
> +        bErr = 0;\
> +        p = strstr(opts, opt "=");\
> +        char* q = strstr(opts, "no-"opt);\
> +        if (p && sscanf(p, opt "=%d" , &i) && param_val != i)\
> +            bErr = 1;\
> +        else if (!param_val && !q)\
> +            bErr = 1;\
> +        else if (param_val && (q || !strstr(opts, opt)))\
> +            bErr = 1;\
> +        if (bErr)\
> +        {\
> +            x265_log(param, X265_LOG_ERROR, "different " opt " setting than given in analysis file (%d vs %d)\n", param_val, i);\
> +            X265_FREE(opts);\
> +            return false;\
> +        }\
> +    }
> +
> +    char *p = NULL, *paramBuf, *opts;
> +
> +    opts = paramBuf =  X265_MALLOC(char, 2000);

white-space

> +    if (!paramBuf)
> +        return false;
> +
> +    size_t pSize = fread(paramBuf, 1, 2000, this->analysisFile);
> +
> +    /* check whether fanout options are compatible */
> +    if (pSize != 2000 && strncmp(paramBuf, "#options:", 9))

it's ok if less than 2000 bytes were fread, so long as the \n is found.
perhaps just check pSize < 9. Pretty sure that && should be ||

> +    {
> +        x265_log(param, X265_LOG_ERROR, "options list in analysis file is not valid\n");
> +        return false;
> +    }

this would generally be ok if

1. 2000 was a #define or enum {} as max possible param size and it was
   actually enforced during the write
2. the actual length of the header is computed and stored next to
   analysisRecordSize, something like:
     analysisHeaderSize = strchr(paramBuf, "\n") - paramBuf;

> +    int i, j;
> +    uint32_t k , l;
> +    bool bErr = false;
> +    paramBuf = strchr(paramBuf, '\n');
> +    if (!paramBuf)
> +    {
> +        x265_log(param, X265_LOG_ERROR, "Malformed analysis file\n");
> +        return false;
> +    }
> +
> +    if (sscanf(opts, "#options: %dx%d", &i, &j) != 2)
> +    {
> +        x265_log(param, X265_LOG_ERROR, "Resolution specified in analysis file is not valid\n");
> +        X265_FREE(opts);
> +        return false;
> +    }
> +    if ((p = strstr(opts, " fps=")) == 0 || sscanf(p, " fps=%u/%u", &k, &l) != 2)
> +    {
> +        x265_log(param, X265_LOG_ERROR, "fps specified in analysis file is not valid\n");
> +        X265_FREE(opts);
> +        return false;
> +    }
> +    if (k != param->fpsNum || l != param->fpsDenom)
> +    {
> +        x265_log(param, X265_LOG_ERROR, "fps mismatch than given in analysis file (%u/%u vs %u/%u)\n",
> +            param->fpsNum, param->fpsDenom, k, l);
> +        X265_FREE(opts);
> +        return false;
> +    }
> +
> +    CMP_OPT_FANOUT("bitdepth", param->internalBitDepth);
> +    CMP_OPT_FANOUT("weightp", param->bEnableWeightedPred);
> +    CMP_OPT_FANOUT("bframes", param->bframes);
> +    CMP_OPT_FANOUT("b-pyramid", param->bBPyramid);
> +    CMP_OPT_FANOUT("b-adapt", param->bFrameAdaptive);
> +    CMP_OPT_FANOUT("open-gop", param->bOpenGOP);
> +    CMP_OPT_FANOUT("keyint", param->keyframeMax);
> +    CMP_OPT_FANOUT("min-keyint", param->keyframeMin);
> +    CMP_OPT_FANOUT("scenecut", param->scenecutThreshold);
> +    CMP_OPT_FANOUT("ctu", (int)param->maxCUSize);
> +    CMP_OPT_FANOUT("ref", param->maxNumReferences);
> +    CMP_OPT_FANOUT("rc-lookahead", param->lookaheadDepth);
> +
> +#undef CMP_OPT_FANOUT
> +
> +    X265_FREE(opts);
> +    return true;
> +}
> +
>  void CLIOptions::readAnalysisFile(x265_picture* pic, x265_param* p)
>  {
>      int poc, width, height;
> @@ -788,7 +874,7 @@
>  
>  void CLIOptions::writeAnalysisFile(x265_picture* pic, x265_param *p)
>  {
> -    uint64_t seekTo = pic->poc * this->analysisRecordSize;
> +    uint64_t seekTo = pic->poc * this->analysisRecordSize + 2000 /* exta bytes used for dumping param */;
>      fseeko(this->analysisFile, seekTo, SEEK_SET);
>      fwrite(&p->sourceWidth, sizeof(int), 1, this->analysisFile);
>      fwrite(&p->sourceHeight, sizeof(int), 1, this->analysisFile);
> @@ -912,6 +998,18 @@
>  
>          cliopt.analysisRecordSize = ((sizeof(int) * 4 + sizeof(uint32_t) * 2) + sizeof(x265_inter_data) * numCU * 85 +
>          sizeof(uint8_t) * 2 * numPart * numCU + sizeof(char) * numPart * numCU + sizeof(int) * numCU  + sizeof(uint32_t) * numCU);
> +        if (param->analysisMode == X265_ANALYSIS_SAVE)
> +        {
> +            char *p = x265_param2string(param);
> +            if (p)
> +                fprintf(cliopt.analysisFile, "#options: %s\n", p);

else
   param->analysisMode = X265_ANALYSIS_OFF;

> +            X265_FREE(p);
> +        }
> +        else
> +        {
> +            if (!cliopt.validateFanout(param))
> +                goto fail;
> +        }
>      }
>  
>      if (cliopt.bDither)
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel

-- 
Steve Borho