<!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>