[vlc-devel] [PATCH] IVTC / Inverse telecine deinterlacer

[Juha Jeronen]

Hi all,

On 04/01/2011 05:49 PM, Juha Jeronen wrote:
> Hi,
>
> On 04/01/11 16:34, Jean-Baptiste Kempf wrote:
>> Hello,
>>
>> On Thu, Mar 31, 2011 at 09:55:26PM +0300, Juha Jeronen wrote :
>>> For some reason, "diff" didn't like my changes - it thinks I rewrote
>>> Deinterlace(), while actually I just inserted a whole lot of code just
>>> before it. This makes CalculateInterlaceScore() in the patch
>>> unnecessarily hard to read. Any ideas to overcome this are appreciated.
>>
>> git diff -C ?
>
> So git format-patch -C <some-number> <base-commit-id> ?
>
> I'll try that for the next version of the patch, whenever that is.
> Thanks :)

Note to self and anyone curious:

git format-patch --patience <base-commit-id>

does the trick.

The end result is still 122kB (as opposed to 128kB), so I'll only repost
this trivially corrected version if you guys think it would be preferable.


Looking at the patch myself today, I noticed the following which I
didn't notice when I was posting it. These can be considered known issues:

- In TestForMotionInBlock(), #ifdef CAN_COMPILE_MMXEXT should surround
all of the MMX code, but there is a trivial mistake. The first #endif
and the next #ifdef should be removed to make it correct.

- In IVTCOutputOrDropFrame(), the asserts checking p_ivtc->i_cadence_pos
and p_ivtc->i_tfd should run only if p_ivtc->b_sequence_valid. Due to
the current implementation, it doesn't matter that they are always
checked in IVTC_MODE_TELECINED_NTSC_HARD, but it's conceptually wrong.

- Initialiation of pi_top_rep and pi_bot_rep in IVTCClearState() is
inconsistent with that in IVTCFrameInit(). The values currently used in
IVTCFrameInit() should be used in both cases.

- Some comments - especially the lengthy one explaining the filter -
have accidentally acquired some extra ( and " at the start of text lines
in manual rewrapping, due to how Shift+Enter works in Kate.

- There is some repetition in the comments. OnceAndOnlyOnce could be
applied here, too.

- This I meant to ask, but forgot: would it be a sensible idea to always
make field duration measurements in Deinterlace()? Currently it is only
done if the chosen algorithm is a framerate doubler, which IVTC is not.
IVTC currently uses its own, cruder estimate of frame duration for the
custom PTS adjustment. See the FIXMEs in IVTCOutputOrDropFrame(). I
might change this, if no one objects.


That's it for now. Maybe I'll now wait for a review, and watch some
anime since the filter works ;)

 -J