[vlc-devel] Offsets, plane_CopyPixels() and deinterlace

[Juha Jeronen]

Hi all,

> On Sun, May 1, 2011 at 4:55 PM, Laurent Aimar <fenrir at elivagar.org
> <mailto:fenrir at elivagar.org>> wrote:
>
>      If you use i_visible_*, you must also use i_?_offset (which
>     define the top-left of
>     position the rectangle inside the video surface).
>

i_x_offset and i_y_offset in video_format_t? Thanks Laurent :)

Deinterlace is missing this. It assumes zero offset everywhere. I'll fix
this later. (Note to self: need to look at least at ComposeFrame() in
helpers.c and DarkenField() in algo_phosphor.c. Possibly also any
algorithms that do format conversions, as the offsets in
source/destination may be different...)


But a more important reason that I'm posting this is that in the core, I
noticed that plane_CopyPixels() (and thus also picture_CopyPixels())
doesn't take offsets into account.

When it's not copying the whole memory region, it's copying
i_visible_pitch pixels from each line, starting at the left edge. If
I've understood things correctly, this copies the left margin, and
leaves out as many pixels from the right edge of the visible area as
there were in the left margin.

This case occurs when the source and destination pitches don't match, or
when handling field plane_t's.


There are two problems here: first, plane_CopyPixels() has no access to
the offset data. Without the offset data, there is no way to optimize
the cropping (i.e. crop margins first) for destinations smaller than the
source.

The copying could be made offset-agnostic by using i_pitch instead of
i_visible_pitch, but this is not a good solution for at least two reasons:

1) copying would still always start at the left edge, so if the
destination is smaller than (source - right margin) it would still leave
in all of the left margin and crop part of the visible area.

2) field plane_t's; recall that plane_CopyPixels() originally worked
somewhat like this, and was changed specifically to account for handling
single fields, too.


This gets us to the second problem. Field plane_t's - used in the
deinterlacer - use i_visible_pitch differently from its usual meaning.
FieldFromPlane() doubles i_pitch, while keeping i_visible_pitch as-is.
Given the current implementation of plane_CopyPixels(), this implies
zero left margin in the original, and it effectively defines the other
field as part of the right margin. This makes it possible to use
plane_CopyPixels() to copy single fields.

This would work correctly, if plane_CopyPixels() took into account the
(original) left margin.

Considering the present use case of field plane_t's only, it would be
possible to hack a temporary solution for this particular problem by
defining i_visible_pitch in a field plane_t as the original i_pitch (so
that plane_CopyPixels() unknowingly copies both margins), but this
sounds very brittle, hacky, and very likely to backfire later.


I think that short of adding the offset information into plane_t, there
is no general solution. (Besides, what's the correct offset in a
subsampled chroma plane? Is it possible to calculate it from the luma
offset, or are they independent?)

Comments?

 -J

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110508/3a04dcee/attachment.html>