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

[Clément Bœsch]

On Fri, Jun 24, 2011 at 10:50:02AM +0200, Erik Slagter wrote:
[...]

The main issue is about the coding style. Some precisions attached.

> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index cc5b983..4bbeb0f 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -198,14 +198,18 @@ 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)

Please break this line (80 columns):

    static int opt_str(xxxx,
                       yyyyy,
                       ...)
    {

> +{
> +    if(!param)

The coding style prefers "if (", even if most the style is non consistent
in that file.


> +        return(1);

return 1;

> +
> +    switch(x264_param_parse(&x4->params, opt, param))
> +    {

switch (...) {

> +        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: { return(1); }

Pointless { }, 80 columns prefered.

> +    }
> +}
> 
>  static av_cold int X264_init(AVCodecContext *avctx)
>  {
> @@ -308,7 +312,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))
> +        return(-1);
> 

ditto, and same below

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110624/f6f172db/attachment.asc>