[vlc-devel] [PATCH] Phosphor deinterlacer, reworked += 4

Juha Jeronen juha.jeronen at jyu.fi
Sun Mar 20 22:03:35 CET 2011


Hi,

Thanks for the comments, and for taking the time to review.

On 03/20/2011 10:23 PM, Rémi Denis-Courmont wrote:
>> - Patch 2: moved "#undef Merge" to after ComposeFrame() (which is now
>> the last function that needs it). This is not as clean as it used to be,
>> but I think this change is not harmful. In any case it's a very minor
>> point, because the issue will become moot with the soon-upcoming
>> refactoring of the module.
> I wonder if the long text is not too long for the user interface (not tested).

In the purely technical sense, it's not. I've tested it, and it displays
completely at least in the Qt GUI.

As for readability, the lack of newlines in tooltips may make it hard to
read.

I'd like to make the text shorter, but the problem is that I don't see
how to explain it any more briefly. I can (will) make one more pass
attempting that. We'll see if having not looked at it for a few days
helps :)

I'd like to keep a fairly complete (but short!) description in the
longtext, because any new strings in the GUI will probably get
localized, while any separate documentation probably won't. Also, this
way it's less likely to go out of date with the code.

> Otherwise, you should really cache vlc_CPU(). The compiler does not know that 
> it's constant. This is a stupid limitation of Windows DLL, which cannot export 
> global variables...

Thanks, good point. Will fix.

I'll wait for Laurent's comments, and then make the remaining fixes for
this in one go.

 -J




More information about the vlc-devel mailing list