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

[Juha Jeronen]

Hei,

On 02/09/11 22:23, Rémi Denis-Courmont wrote:
> 	Moi,
>
> Le mercredi 9 février 2011 22:00:05 Juha Jeronen, vous avez écrit :
>> Here is a new version of my patch #2 (repeat_pict support) based on
>> Laurent's comments. This replaces my earlier posted patch #2.
>> Changes with regard to previous version:
>>
>> - Do not make assumptions about frame delay (offset), but let the
>> algorithms decide. This enables Yadif2x to work correctly.
>> - Compute output PTSs in Deinterlace() in a centralized way.
>> - Use a for loop for output frame alloc and PTS setting.
>>
>> The frame offset mechanism, along with its rationale, is documented in
>> the source, just below the filter_sys_t structure.
> Nice to have documentation actually.
That's good to hear. I usually document my stuff at least on some level :)

> But but this file is really getting huge... it really needs to be (sensibly) 
> sliced before it gets expanded much further, I think.

I agree. If the latest IVTC is included, this file will balloon to
something like 4500 lines.

But before I do anything drastic, I'd like to hear Laurent's comments on
the issue, too :)


Based on what I currently know about VLC's deinterlacer, I would
tentatively suggest the following split:

- Consider deinterlace.c as the main source file for the deinterlacer
subsystem. Keep Deinterlace(), Open(), Flush(), Close(),
SetFilterMethod(), GetOutputFormat() etc. general stuff in that one.

- filter_sys_t: this definition needs to be seen by all parts of the
deinterlacer. Move to header?

- Simple rendering algorithms with just one function, or maybe two
functions at most: make a new file that gathers these.

- More complex rendering algorithms with lots of helper functions (e.g.
X) or private data (e.g. the upcoming IVTC): one file per algorithm.

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


 -J