[MPlayer-dev-eng] [PATCH] new vf_ass implementation

[Zongyao Qu]

I am reviewing this as far as I could...

>  static int config(struct vf_instance *vf,
>                    int width, int height, int d_width, int d_height,
>                    unsigned int flags, unsigned int outfmt)
>  {
>      struct mp_eosd_settings res = {0};
> +    struct dirty_rows_extent *dirty_rows;
> +    int outw, outh;
> +    int planes, alphas;
> +    int i;

>      vf->priv->outfmt = outfmt;
> -    vf->priv->outh = height + ass_top_margin + ass_bottom_margin;
> -    vf->priv->outw = width;
> +    vf->priv->outh = outh = height + ass_top_margin + ass_bottom_margin;
> +    vf->priv->outw = outw = width;
> 
> -    if (!vf->priv->is_planar)
> -        vf->priv->planes[0] = malloc(vf->priv->outw * vf->priv->outh);
> -    vf->priv->planes[1]  = malloc(vf->priv->outw * vf->priv->outh);
> -    vf->priv->planes[2]  = malloc(vf->priv->outw * vf->priv->outh);
> -    vf->priv->dirty_rows = malloc(vf->priv->outh);
> +    for (i = 0; i < planes; i++)
> +        vf->priv->planes[i] = malloc(outw * outh);
> +    for (i = 0; i < alphas; i++)
> +        vf->priv->alphas[i] = malloc(outw * outh);
> +    dirty_rows = malloc(outh * sizeof(*dirty_rows));
> +    for (i = 0; i < outh; i++) {
> +        dirty_rows[i].xmin = 0;
> +        dirty_rows[i].xmax = outw;
> +    }

1.
what is the purpose of the local variables outw/outh in config()?
why not using vf->priv->outh/outw directly?

2.
The other planes/alphas pointers which are not used, better set to NULL.
This is the only place to initialize the vf_priv_s, I am not sure 
in all platforms they are initialized to NULL, otherwise it will crash
in uninit().

Or alternatively, in m_struct.c:60, the vf->priv is initialized once
by copied from vf_priv_dflt, so as a better solution, we could add the 
initial value to vf_priv_dflt which is located in vf_ass.c:74

>  static int put_image(struct vf_instance *vf, mp_image_t *mpi, double pts)
>  {
>      struct mp_eosd_image_list images;
>      eosd_render_frame(pts, &images);
>      prepare_image(vf, mpi);
> -    render_frame(vf, mpi, &images);
> +    if (images.changed)
                  ^^^^^^^
Is there any docs you could find on how this flag works?
I know nothing about it.
> +        prepare_eosd(vf, &images);
> +    vf->priv->render_frame(vf);
>      return vf_next_put_image(vf, vf->dmpi, pts);
>  }
>