[MPlayer-dev-eng] [PATCH 2/2] av_helpers: show libav* version and configuration.

[Reimar Döffinger]

On Wed, Dec 21, 2011 at 03:59:30PM +0100, Nicolas George wrote:
> +static void show_av_version(int type, const char *name,
> +                            int st, int dl, const char *conf)
> +{
> +    char buf[128];
> +    int l = 0;
> +
> +    snprintf(buf + l, sizeof(buf) - l, "Init %s version %d.%d.%d", name,
> +             st >> 16, (st >> 8) & 0xFF, st & 0xFF);
> +#ifdef CONFIG_FFMPEG_SO
> +    l += strlen(buf + l);
> +    snprintf(buf + l, sizeof(buf) - l, " (dynamic");
> +    if (dl != st) {
> +        l += strlen(buf + l);
> +        snprintf(buf + l, sizeof(buf) - l, " version %d.%d.%d",
> +                 dl >> 16, (dl >> 8) & 0xFF, dl & 0xFF);
> +    }
> +    l += strlen(buf + l);
> +    snprintf(buf + l, sizeof(buf) - l, ")");
> +#endif
> +    mp_msg(type, MSGL_INFO, "%s\n", buf);
> +    mp_msg(type, MSGL_V, "Configuration: %s\n", conf);
> +}

Quite a few nits.
First, function is called show_av_version but it prints "Init".
IMO that's confusing, it could just drop the "Init" part.
Next, it would be far more readable without all the snprintf.
Something like

#ifdef CONFIG_FFMPEG_SO
#define FFMPEG_TYPE "external"
#else
#define FFMPEG_TYPE "internal"
#endif

mp_msg(type, MSGL_INFO, "%s version %d.%d.%d ("FFMPEG_TYPE")\n", name, ver >> 16, (ver >> 8) & 0xff, ver & 0xff);
if (ver != header_ver)
  mp_msg(type, MSGL_INFO, "Mismatching header version %d.%d.%d\n", header_ver >> 16, (header_ver >> 8) & 0xff, header_ver & 0xff);
mp_msg(type, MSGL_V, "Configuration: %s\n", conf);

Note that IMHO the names "st" and "dl" are not ideal.
They are really the header and code versions respectively.
And while it really should never ever happen with internal FFmpeg
printing a mismatch is a good idea (in both cases it should
be harmless most of the time, but will be a pain to debug if not).
I also change static/dynamic to internal/external, because static or
dynamic is independent of this (even if the internal+dynamic version
does not exist and external+static will probably need some hacks to
get working).