[vlc-devel] [PATCH] Phosphor deinterlacer reworked += 2

[Juha Jeronen]

Hi all,

This took a bit longer than expected, mostly due to the problem with the
deinterlacer mentioned in the other post. Since I'm quite confident this
is not the cause, I'm posting the updated patch.

Thus:

Attached is the updated Phosphor deinterlacer patch, reworked according
to the review by Laurent. Thanks for the helpful comments.

This version is hereby submitted for a new review.

Changelog:

- Phosphor config values changed to integers
- Working area (render buffer) completely eliminated
- Use substructure for algorithm-specific state
- Test order fixed in GetOutputFormat()
- ComposeFrame() rewritten:
  - plane_ToFieldPlane() added for converting a frame plane_t into a
field plane_t
    - Note: proposing including this into picture.c / libvlccore,
because related stuff is already there.
  - plane_CopyPixelsLineByLine() added (picture.c, vlc_picture.h /
libvlccore)
    - Proposing the addtion of this, because the regular version detects
matching pitches and does a block copy, overwriting the other field in a
field plane_t. Details in the patch.
  - These new functions are used in the new ComposeFrame().
  - CC_MERGE mode uses the smallest visible pitch.
  - New version does not need matching pitches in any of the modes.
- DecayLuma() rewritten:
  - Renamed to DarkenField() (more descriptive after the algorithm change)
  - In-place processing only (much simpler)
  - Pointer incrementation sanitized
  - Useless comment removed
  - Darken also chroma for 4:2:2
  - Implemented MMX versions for both luma and chroma handling
  - Proposing to leave the useless casts in after all, because omitting
them would cause a compiler warning. A warning would suggest there may
be a mistake; casting explicitly, OTOH, says that yes, this is what was
intended.
- RenderPhosphor:
  - Assert in "cannot happen" default case changed to assert(0);

As for my own observations, mentioned in an earlier post:

- The FIXME for pitch matching has been resolved. It's no longer
relevant in DecayLuma/DarkenField(), but now I know what to do in
similar situations. Thanks Laurent :)
- The FIXME for YV12 - I thought about it a bit, and since it's the
filter that's doing the chroma conversion - yes, it needs to swap the
chroma planes in the upconversion from YV12 to I422. Since the logic is
correct, FIXME removed.
- Phosphor-related comments, help texts, config description strings,
etc. updated.
- Default 4:2:0 chroma mode is now AltLine, keeping in mind the main use
case the function comment advertises Phosphor for.
- DecayLuma/DarkenField(): now there are remove_high_u64 and
remove_high_u8 to be semantically correct.

Left as-is:

- Couldn't make the help text shorter.
- DecayLuma/DarkenField(): the loop handling the last pixels in every
line. Shouldn't really matter.
- Duplication between compose_chroma_t and phosphor_chroma_t.
Semantically different, good enough.

The new ComposeFrame() made it possible to eliminate the working area,
which simplifies things a bit. IVTC won't need it, either, once I update
CalculateInterlaceScore() :)

 -J

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Phosphor-deinterlacer.patch
Type: text/x-patch
Size: 45697 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110304/192bccea/attachment.bin>