[MPlayer-dev-eng] [PATCH] -vo lavf

[Reimar Döffinger]

On Thu, Oct 28, 2010 at 10:05:03PM +0200, Nicolas George wrote:
> Note that I needed to change vf_vo so that VOCTRL_DRAW_IMAGE would get the
> pts as an additional argument.

I'd prefer you didn't, I'd prefer to save myself having to think
if there might be any compatibilty issue with some calling conventions.
Also, while it is a hack there is already a vo_pts variable.
Lastly, I miss the most important information: why?
What is the big advantage over using mencoder?

> +static AVRational parse_rational(const char *str)
> +{
> +    AVRational r;
> +    double d;
> +    char dummy;
> +
> +    if (sscanf(str, "%d/%d%c", &r.num, &r.den, &dummy) == 2)
> +        return r;
> +    if (sscanf(str, "%lf%c", &d, &dummy) == 1)
> +        return av_d2q(d, 30000); /* must accept 29.97003 */
> +    r.num = r.den = 0;
> +    return r;
> +}

I'm sure there are already several functions that do that kind
of parsing, I'd prefer not to have yet another one.

> +static int preinit(const char *arg)
> +{
> +    avcodec_register_all();
> +    av_register_all();

Wasn't this FFmpeg initialization code moved to some common code or something?

> +        if (!fps.num || !fps.den) {
> +            mp_msg(MSGT_VO, MSGL_ERR, "vo-lavf: invalid fps\n");
> +            return -1;

What about < 0?

> +    if (!encoder) {
> +        mp_msg(MSGT_VO, MSGL_ERR, "vo-lavf: encoder not found\n");
> +        free_avf();
> +        return -1;
> +    }
> +    if (avcodec_open(video_str->codec, encoder) < 0) {
> +        mp_msg(MSGT_VO, MSGL_ERR, "vo-lavf: unable to open encoder\n");
> +        free_avf();
> +        return -1;
> +    }
> +    if (av_set_parameters(avf, NULL) < 0) {
> +        mp_msg(MSGT_VO, MSGL_ERR, "vo-lavf: unable to set parameters\n");
> +        free_avf();
> +        return -1;
> +    }
> +    if (!(output_format->flags & AVFMT_NOFILE)) {
> +        if (url_fopen(&avf->pb, filename, URL_WRONLY) < 0) {
> +            mp_msg(MSGT_VO, MSGL_ERR, "vo-lavf: could not open '%s'\n",
> +                   filename);
> +            free_avf();
> +            return -1;
> +        }
> +    }
> +    if (av_write_header(avf) < 0) {
> +        mp_msg(MSGT_VO, MSGL_ERR, "vo-lavf: could not write header\n");
> +        free_avf();
> +        return -1;
> +    }

I consider a goto vastly preferable over this code-quituplication...