[FFmpeg-devel] [PATCH 03/11] ffserver_config: map ffserver options to AVOptions

[Reynaldo H. Verdejo Pinochet]

Hi

On 11/16/2014 10:46 PM, Lukasz Marek wrote:
> Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> [..]
> @@ -965,43 +881,38 @@ static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd,
>          ret = av_parse_video_size(&w, &h, arg);
>          if (ret < 0)
>              ERROR("Invalid video size '%s'\n", arg);
> -        else if ((w % 2) || (h % 2))

See bellow

> -            WARNING("Image size is not a multiple of 2\n");
> -        if (av_dict_set_int(&config->video_conf, "VideoSizeWidth", w, 0) < 0 ||
> -            av_dict_set_int(&config->video_conf, "VideoSizeHeight", h, 0) < 0)
> -            goto nomem;
> -    } else if (!av_strcasecmp(cmd, "VideoFrameRate")) {
> -        AVRational frame_rate;
> -        ffserver_get_arg(arg, sizeof(arg), p);
> -        if (av_parse_video_rate(&frame_rate, arg) < 0) {
> -            ERROR("Incorrect frame rate: %s\n", arg);
> -        } else {
> -            if (av_dict_set_int(&config->video_conf, "VideoFrameRateNum", frame_rate.num, 0) < 0 ||
> -                av_dict_set_int(&config->video_conf, "VideoFrameRateDen", frame_rate.den, 0) < 0)
> +        else {
> +            if ((w % 2) || (h % 2))

Drop the redundant () across %. Also, please make an effort
to break lines at 80 chars as long as it doesn't make the
code harder to read. This seems particularly possible on the
function declarations.

Other than these two minor nits, the patch seems OK
to push.

Thanks a lot.

-- 
Reynaldo H. Verdejo Pinochet
Open Source Group
Samsung Research America / Silicon Valley