[vlc-devel] [PATCH] Soft field repeat support for deinterlacer

[Juha Jeronen]

On 02/11/11 19:39, Jean-Baptiste Kempf wrote:
> On Thu, Feb 10, 2011 at 01:55:29PM +0200, Juha Jeronen wrote :
>> Based on what I currently know about VLC's deinterlacer, I would
>> tentatively suggest the following split:
> Yes.
---8<---[snip]---8<---

Ok.

>> - Maybe one file for common helper functions? I have such a case coming
>> up: both IVTC and Phosphor need ComposeFrame().
> Maybe this is overkill.

Maybe :)

I was thinking that the Merge() variants might be useful for more than
just RenderBlend() in the future, in a way similar to how Phosphor
benefited from having ComposeFrame() easily accessible.

OTOH, I'm not sure exactly what they could be useful for, so it was more
a thought of keeping them easily accessible to provide building blocks
for future algorithms. This would enable easily testing new ideas that
need different kinds of low-level operations (and avoid code duplication
if the same subproblem has already been solved). The main idea being
that if it's relatively low effort to try out new ideas, it'll actually
get done :)

> However, I would propose to split AFTER the commits of the IVTC (and
> phosphor)

This would indeed get the new filters in faster, and require less
reworking of the patches.

I can help with the splitting after these are in, of course.

> And I would also suggest that you resend a clean batch for the IVTC and
> Phosphor filters.

Will do. I haven't yet re-formatted the IVTC patch to work with the
updated #2. I thought I'd wait for Laurent's comments on the new #2, and
once #1 and #2 are finalized, then update the IVTC patch.

Myself, I just noticed some minor things to fix in the new #2:

- pb_top_field_firsts is not properly initialized, until it fills up
when history entries are written. The uninitialized elements are never
accessed, but for completeness it'd be reasonable to init this, too.

- code duplication instead of for loop for setting b_progressive to true
for all output frames. Should change this for consistency with the rest.

- code duplication instead of for loop for picture_Release() upon
failure. Like above.

Posting these so that they can be considered known issues. I'll fix them
in the next version.

In the IVTC patch, if it's ok, I'd like to include the new cadence
detector in the next version. In any case, there's some reworking to do
(to conform to the new patch #2), so it's not a major thing to update it
while I'm at it.

 -J