<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#ffffff">
Hi all,<br>
<br>
<blockquote
cite="mid:BANLkTi=Q83YaX6mb7NHR4M74sgkYcOKvBA@mail.gmail.com"
type="cite">
<div>
<div class="gmail_quote">On Sun, May 1, 2011 at 4:55 PM, Laurent
Aimar <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:fenrir@elivagar.org">fenrir@elivagar.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt
0.8ex; border-left: 1px solid rgb(204, 204, 204);
padding-left: 1ex;"> If you use i_visible_*, you must also
use i_?_offset (which define the top-left of<br>
position the rectangle inside the video surface).<br>
</blockquote>
</div>
</div>
</blockquote>
<br>
i_x_offset and i_y_offset in video_format_t? Thanks Laurent :)<br>
<br>
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...)<br>
<br>
<br>
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.<br>
<br>
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.<br>
<br>
This case occurs when the source and destination pitches don't
match, or when handling field plane_t's.<br>
<br>
<br>
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.<br>
<br>
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:<br>
<br>
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.<br>
<br>
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.<br>
<br>
<br>
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.<br>
<br>
This would work correctly, if plane_CopyPixels() took into account
the (original) left margin.<br>
<br>
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.<br>
<br>
<br>
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?)<br>
<br>
Comments?<br>
<br>
-J<br>
<br>
</body>
</html>