[MPlayer-dev-eng] [PATCH] Add IMGFMT_444P and IMGFMT_422P to vo_md5sum.c

[Reimar Döffinger]

On Sun, Jun 06, 2010 at 08:02:48PM +0200, Giorgio wrote:
> Reading the code I got the impression that mpi->width and mpi->w have
> always the same value (when an image is created in new_mp_image(),
> these values are initialized like this: mpi->width=mpi->w=w;). The
> same happens with mpi->height and mpi->h. So I used mpi->width and
> mpi->height because I think it's more clear. Is this correct?

Oh, I know people got used to MPlayer having no useful documentation,
it still is a good idea to look for it...
    int width,height;  // stored dimensions
    int x,y,w,h;  // visible dimensions
So no, it is plain and simple wrong.

> @@ -195,8 +195,8 @@
>  static uint32_t draw_image(mp_image_t *mpi)
>  {
>      unsigned char md5sum[16];
> -    uint32_t w = mpi->w;
> -    uint32_t h = mpi->h;
> +    uint32_t w = mpi->width;
> +    uint32_t h = mpi->height;

And that this is more clear is a more than questionable claim IMO.
That is completely independent from the fact that such cosmetic changes
(well, if it was one) would have to be in a separate patch, and in
addition if they were the same the right solution would be to remove one,
not randomly use one in some parts of the code and the other in other
parts of the code.

> @@ -215,8 +215,8 @@
>              for (i=0; i<h; i++) {
>                  av_md5_update(md5_context, planeY + i * strideY, w);
>              }
> -            w = w / 2;
> -            h = h / 2;
> +            w = mpi->chroma_width;
> +            h = mpi->chroma_height;

For the same reason you can't use these here, you'll have to explicitly
shift by chroma_?_shift