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

[Jindrich Makovicka]

D Richard Felker III wrote:
> On Tue, Nov 23, 2004 at 08:57:15AM +0100, Jindrich Makovicka wrote:
> 
>>Hi,
>>
>>the attached patch modifies the *_get_image to make possible to specify 
>>an explicit horizontal and vertical alignment. This issue has already 
>>been discussed in the thread
>>
>>http://mplayerhq.hu/pipermail/mplayer-dev-eng/2004-October/030502.html
>>
>>The patch
>>
>>1) renames vf_get_image to vf_get_image_aligned and adds two additional 
>>parameters, which specify alignment_bytecount-1 (only 2^n-1 are valid)
>>
>>2) creates vf_get_image as wrappers calling vf_get_image_aligned with 
>>the proper alignment parameters - height_align=0, width_align=either 15 
>>or 0 depending on MP_IMGFLAG_ACCEPT_ALIGNED_STRIDE.
>>
>>3) changes all (they are only a few) occurences of vf_get_image and 
>>mpcodecs_get_image with aligned dimensions to calls of 
>>_get_image_aligned with original dimension and explicit specification of 
>>the alignment.
> 
> 
> could you please explain why this is necessary? i was under the
> impression it was just a bug in the codec. stride alignment is already
> done, anyway...

as i wrote in cvslog, the current api doesn't allow to allocate aligned 
images correctly - you can only choose, where you prefer the access 
violation to occur. if you allocate the exact picture size without 
safety bands, it can crash in the decoder. with the safety bands, it can 
crash in the VO.

when the codec calls get_image with the enlarged size, the elnarged mpi 
can be eventually propagated up to the VO, which was set up with the 
original (smaller) size and already allocated its buffer to suit it. 
this occurs unless you use slices, where the codec's draw_slice usually 
does the bounds checking. another approach is to do boundary checks in 
every VO, which is obviously painful, as the VO's handling of the images 
is a now inconsistent enough to introduce further ad-hocism.

> with that said, i'd like to recommend a different api. rather than
> requesting alignment of allocated width/height, the caller should
> request a (minimum) amount of 'border space' it wants writable on all
> sides of the image, including top and left. this type of design works
> even with more complex filter chains.

sounds reasonable and the code can be easily adapted.

> furthermore, ALL of this (image size, border, etc.) should be decided
> at CONFIG TIME, not everytime get_image is called. get_image should
> implicitly assume the same sizes config() was called with, otherwise
> it's nonsense. of course this would be a major pain to fix up in g1...
> let's discuss it more.

what about vf_scale, vf_expand etc. ? maybe config() could also return 
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.

Regards,
-- 
Jindrich Makovicka