[MPlayer-dev-eng] [PATCH] changes in vf semantics

[D Richard Felker III]

On Wed, Dec 01, 2004 at 03:18:19AM +0200, Ivan Kalvachev wrote:
> On Tue, 23 Nov 2004 21:56:10 +0100
> Reimar D_ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> 
> > Hi,
> > > > [...]
> > > > > > the desired destination size or just write it somewhere to vf_instance_s 
> > > > > > and let it be. get_image would be then called with the current and next 
> > > > > > vf as argument, instead of next and x/y-size.
> > > > > 
> > > > > again you totally misunderstand the video layer. i don't have time to
> > > > > test and debug this stuff right now, but imo someone like myself,
> > > > > ivan, or michael who understands it should be the one to fix it,
> > > > > rather than you adding crazy hacks based on misunderstandings.
> > > > 
> > > > Oh, I'm sure we would not mind that. If only it looked like this is
> > > > ever going to happen. This bug is know at least since months, and
> > > > nobody gave even any useful comment.
> > > 
> > > could you do some research then and find out why it doesn't happen
> > > with vd_ffmpeg.c?
> > 
> > No, I can't because it does happen with vd_ffmpeg.c as well (maybe not
> > in all cases though). See my second patch in the "segfault in vf_scale"
> > thread.
> 
> It usually doesn't happen with normal ffmpeg codecs because most of them
> support emu_edge, this they don't need alignment.
> 
> The problem is real and quite messy.
> Actually there are few problems:
> 
> First. Alignment
> 
> The horizontal line alignment is not an issue as it is easily to workaround by flags.
> The real problem is vertical block alignment, as it is not specified anywhere.
> 
> As really really messy workaround we could make 16 lines height alignment when 
> horizontal flag alignment is set.
> 
> Second.
> Video initialization.
> By legacy reasons the vo is initialized before even first mpi structure is filled.
> This makes vo_xv and probably other vo allocate memory, init devices
> even before knowing what alignment they may need.

This is nonsense. Yes it's true, but in this case direct rendering can
just fail, no problem. The vo's "get_image" implementation can see
the requested width/height, and if they don't match the configured
ones, it can just refuse to do dr. The normal non-dr functions of the
vo DO NOT CARE about stride/alignment crap for their source image.

Granted this is sub-optimal in terms of performance, but it only
affects shitty files, and it's STILL much better than the current
situation where these files cause sig11!! Also it's a minimal change
so we can do something better later if we want.

> Because of this, we should pass the aligned dimensions to vo config().
> But this may lead to problems as we also pass visible dimensions (aspect) at the same time.
> (e.g. off by few lines creates horrible scaling artifacts on cheap old video cards,
> like the one I used to have). For fixed size vo it may lead to black bars, but this is not really issue.

This is totally broken and unacceptable. There's a reason it's not
implemented that way.

> Finally.
> 
> Probably it would be good idea to add an alignment parameters to the mpcodecs_config_vo()
> and remove them from get_image() (e.g. making flags informative, not settable).
> This seams to be also the less intrusive way.
> Indeed, it will need changing of all vo-s.

More total nonsense. There's no reason special alignment/border needs
should be restricted to codecs! Filters might need it too!!!

> Probably an better way to fix it is to pass mpi structure to vo_config(),
> as mpi already have both visible and "physical" dimensions and alignment info.
> An better and more generic alignment parameters could be added.
> Probably adding aspect info to mpi would be interesting change,too ;)

What you're talking about is a whole new vf api, so go write the whole
thing in a non-broken way and call us in 5 years when you're done.
Meanwhile we're trying to FIX MPLAYER G1 WITH MINIMAL CHANGES TO VF SO
THAT IT NEVER CRASHES ON STRANGE-SIZED MOVIES!! And the proposed fix
does exactly that.

> I insist to discuss this here in maillist as it give more time to think about issues,
> more time to check the code and test things.

OK, we'll give you the 5 years to write g3 vf... ;)
But each week it takes adds 10l cola to your tab. :))))

> p.s.
> i wrote this mail right before going to sleep, if I overlooked
> something, point it kindly;)

That wouldn't be any fun... ;)
We need some flames now and then.

Rich