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

[Giorgio]

2010/6/8 Reimar Döffinger <Reimar.Doeffinger at gmx.de>:
> On Tue, Jun 08, 2010 at 05:18:42PM +0200, Giorgio wrote:
>> Yes, I looked there, but I didn't understand completely what "stored
>> dimensions" and "visible dimensions" meant in that context.
>
> "stored dimensions" means basically how much memory is allocated,
> the "visible dimensions" indicates how much of that is supposed
> to be visible.
> A stored dimension od 320x240 with a visible dimension of 320x200
> would mean the last 40 lines should not be displayed.

Yes, now I see what you mean.

> I agree with you though that it's a mess and hard to understand
> because (probably) quite a bit of code does not do this distinction
> properly (mostly because for filters it does not matter or might
> even be better to ignore the "visible area" cropping.

Exactly...from what I've seen these "visible dimensions" don't work as
expected (I will look again at the code sometimes). I tried this
approach when I wanted to fix the offsetting in theora videos.

>> There are lines like this: "vf.c: MPI parameters changed!  400x240 ->
>> 320x240", but also in vf.c there is this piece of code for handling
>> this case:
>>
>>       if(mpi->flags&MP_IMGFLAG_ALLOCATED){
>>           if(mpi->width<w2 || mpi->height<h){
>>               // need to re-allocate buffer memory:
>>               av_free(mpi->planes[0]);
>>               mpi->flags&=~MP_IMGFLAG_ALLOCATED;
>>               mp_msg(MSGT_VFILTER,MSGL_V,"vf.c: have to REALLOCATE buffer memory :(\n");
>>           }
>> //    } else {
>>       } {
>>           mpi->width=w2; mpi->chroma_width=(w2 + (1<<mpi->chroma_x_shift) -
>> 1)>>mpi->chroma_x_shift;
>>           mpi->height=h; mpi->chroma_height=(h + (1<<mpi->chroma_y_shift) -
>> 1)>>mpi->chroma_y_shift;
>>       }
>>     }
>>
>> so it might work just fine.
>
> You are getting close.
> Let's take some different parts of this function:
>
>  assert(w == -1 || w >= vf->w);
>  assert(h == -1 || h >= vf->h);
>  assert(vf->w > 0);
>  assert(vf->h > 0);
> [...]
>  if (w == -1) w = vf->w;
>  if (h == -1) h = vf->h;
> [...]
>  mpi->w=vf->w; mpi->h=vf->h;
> [...]
>            mpi->width=w2; mpi->chroma_width=(w2 + (1<<mpi->chroma_x_shift) - 1)>>mpi->chroma_x_shift;
>            mpi->height=h; mpi->chroma_height=(h + (1<<mpi->chroma_y_shift) - 1)>>mpi->chroma_y_shift;
>
> w and h are set to vf->w and vf->h, which are the actual dimensions that should be visible
> (i.e. processed by the vos), whereas width/height can be larger (to enable certain
> optimizations).

I had noticed that. If I'm not wrong, everything starts in:

int mpcodecs_config_vo(sh_video_t *sh, int w, int h, unsigned int
preferred_outfmt)
[...]
    if (w)
        sh->disp_w = w;
    if (h)
        sh->disp_h = h;
[...]
    vf->w = sh->disp_w;
    vf->h = sh->disp_h;
    if (vf_config_wrapper
        (vf, sh->disp_w, sh->disp_h, screen_size_x, screen_size_y, vocfg_flags,
         out_fmt) == 0) {
//                      "MPlayer",out_fmt)){
        mp_msg(MSGT_CPLAYER, MSGL_WARN, MSGTR_CannotInitVO);
        sh->vf_initialized = -1;
        return 0;
    }

when we configure the filter chain the values of vf->w and vf->h gets
initialized to the proper values (thanks to vf_next_config() ). Then,
when vf_get_image() gets called we use these values with the mpi image

mpi->w=vf->w; mpi->h=vf->h;

but if some filter needs a bigger/smaller image than we already have,
we calculate them again:

if(mpi->width!=w2 || mpi->height!=h){
{
mpi->width=w2; mpi->chroma_width=(w2 + (1<<mpi->chroma_x_shift) -
1)>>mpi->chroma_x_shift;
mpi->height=h; mpi->chroma_height=(h + (1<<mpi->chroma_y_shift) -
1)>>mpi->chroma_y_shift;
}
}

(and try to allocate memory again if we need a bigger image). I'll
read that part again.

Another thing: while reading vf.c I noticed that
vf_default_query_format() isn't used anywhere else in the code. Just
out of curiosity, could it be possible to simplify the code with the
patch attached?

>> So Reimar, if you think it could be an
>> useful addition, please fix it and commit it. If not, sorry for the
>> noise. Thank  you.
>
> I'll try to find the time, thanks for trying :-)

You're welcome :)

Giorgio Vazzana
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vf_default_query_format.diff
Type: text/x-diff
Size: 1283 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100608/79ec02c2/attachment.diff>