[MPlayer-dev-eng] [PATCH] mp_image flags in filters

[Ivan Kalvachev]

On 7/28/05, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> Hi,
> On Thu, Jul 28, 2005 at 01:04:44AM +0300, Ivan Kalvachev wrote:
> > On 7/27/05, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> > > For tile I think MP_IMGTYPE_STATIC is not neccessary, as I understand it
> > > you'd only need that if you call vf_next_put_image more than once with
> > > the same mp_image, which is not the case here.
> >
> > I would preffer this to stay static. Don;t forget that tile is called
> > multiple times over same (result) image
> 
> Well, and when you do memcpy you write into it multiple times as well
> *g*. No, seriously, what is the criteria for using static? I thought
> simply that you can use the same mp_image for multiple
> vf_next_put_image calls (i.e. without having to call get_image again).

STATIC is used when the buffer should not be moved in the memory. In
other means it could not be reallocated. This is was created for some
VFW decoders that cannot change their output buffers.
 
> > boxblur, doesn't seem to support direct rendering. It doesn't
> > implement get_image funtion. So setting readable in the dest image is
> > bad (will actually prevent DR in valid case). I guess that the
> > algorithm requires not to modify the source image.
> 
> ??? Why would setting readable prevent DR?? It would only prevent DR
> into video RAM, which is correct, video RAM is _much_ too slow, even for
> reading just a few bytes. At least that's how I understood it.

I didn't spot where boxblur reads from the gotten image. That's why I
don't think READABLE belongs to this filter. Well if I am wrong.. show
me the code :)

 
> > dint - MAXROWSIZE should be commited separately;)
> 
> Yes, sure, that slipped in.

:)
 
> > divtc - commit that, it does reading and the other get_image() do use readable.
> 
> I really thought the readable in get_image is something completely
> different, it forces the mp_image you get as _input_ to be readable,
> whereas readable in vf_get_image concerns the output image (i.e. the one
> you write into).
> Or what do I misunderstand here?

READABLE means that the filter will perform not only write but also
read operations in this buffer. (e.g. like smartblur, that uses
sws_scale to blur and then compares original and result images).
So if next filter is vo and it have direct access to video memory that
is slow on reading, then vo will not give direct buffer and vf will
use mem-allocated one instead.

> > softpulldown - I don't see why are you removing PRESERVE flag. This
> > filter obviously doesn't want to allow next filters to mess the image,
> > as it needs it on next pass.(btw this also mean that vo cannot draw
> > osd in it;) It seems static and preserve is valid trick for storing
> > image.
> 
> Ok, I understood that code now (sorry, it's rather confusing at the
> first look, and I missed the fact that the stride was multiplied by 2
> *g*

;)
It is easier to spot it if you know what the filter does:) 

> > telecine - same as above. (and this is rich filters)
> >
> > tinterlace - same as softpulldown. But I agree that READABLE is
> > probably not needed here. We don't read from destination buffer.
> 
> Same. READABLE shouldn't be needed in all thiree cases or am I missing
> something again?

I don't see  READABLE added/removed in your patch about
softpulldown,telecine filters ;) I think these filters only write in
gotten buffer so they stay as they are.

> And am I right that static doesn't mean that the pointer must be the
> same but just that it will be the same buffer as the last time?

Same buffer at same place with same pointer.

> But this would also mean, that static without preserve wouldn't make any
> sense?
Preserve is about content, it should not allow next filter to reuse
the same buffer and draw in it.

> Because I'm still not sure if I got vo_gl right...

Don't worry. Only arpi could understand the whole video system at
once. But I am afraid he already had forgot what the code does :O

> Hmmm... when we're finished with this, who volunteers on making some
> DOCS out of it? *g*

No, It would be simplier to write new video system.
I'm somewhere in the beggining still.
 
> Having fun extracting the docs out of the inconsistent code *g*,

look at vf.c for the most embarasing parts.
e.g. every filter have list with the mpi it could hold.
they type of mpi shows where does it belongs
So.
there is one temp buffer at time (one B frame)
there culd be 2 static buffers (P frames)
and one export buffer.
The TEMP/STATIC/EXPORT work with only one buffer.
IP works with 2 static buffers (they get the first then the other)
IPB works with 2 static and one temp (depending on READABLE)

Well I guess you now know what's the problem of h.264 decoders that
can have 16 prediction frames.

Happy Coding 
  Ivan Kalvachev
 iive