Re: Fwd: [xine-devel] [PATCH] zoom in xine-ui/vidixfb

[Hans-Dieter K. <hd...@t-...>]

Hi,

mike lewis wrote:
>>>>So I'll attach the patch and I'm happy to change the patch to meet
>>>>the xine-devel required standards, but I have no idea what they are..
>>>>
>>
>>To learn about the standards, view the xine sources and do the same.
>>
>>Some hints:
>>
> 
> Thankyou.. I appreciat the response ;-).  I actually don't code often
> (not for years) and I don't think of myself as someone who *can* code.
>  All I can do is *massage* the existing code to a little more that it
> did before, but only when I have seen the code required to perform the
> functions written before.. Which bring me to address your kind hints.
> 
Any help is appreciated!
> 
>>   Declare local variables inside the block where they are used.
>>
> 
> Are you talking about "int zoom_x"?  By block you mean outside the
> function/inside the file?  The reason I declared it there was as its
> only required by the function itself, and this seems to be the
> preceding standard for "actions.c ".  If this is not what you mean
> then could you spell it out for me?
> 
You did nothing wrong, sorry for confusing you. It is just a general 
hint. A block is delimited by braces ({}). E.g. if a variable is only 
needed inside an if-block, declare it at the beginning of the block, not 
outside (after the opening "{", not before the "if"). But maybe that was 
already clear to you ;-)
> 
>>   Surround strings to display in gui by _() to make them available for
>>   translation into other languages (see osd_draw_bar()), so your patch
>>   doesn't comply.
>>
>>   If a logging message is not overall important, use printf() instead of
>>   fprintf(stderr, ...); so it's only displayed when xine-ui is run with
>>   verbosity. Use understandable messages, e.g. "blah" doesn't say much.
>>
> 
> Great, I always wondered why _() was used.  However, the line was only
> needed for debug purposes.  I will remove it...
> 
Here I must correct myself. I didn't attend. The _(), a macro defined in 
src/xitk/i18n.h, applies to xitk, not fb; you'd get a compilation error 
if you'd use it. Same for verbosity: not in fb, printf() and fprintf() 
are always displayed.
> 
>>In summary, your patch looks not bad at first glance!
>>
> 
> Oh yay!  If this patch gets accepted then I will finally have given
> somethine back..

Thanks. If there are no objections, I can commit.

> Although I'm a little confused as to why its hasn't
> been done before now.. It seems like a feature people would use/need
> if they were using vidixfb on a regular basis.
> 
Can't judge about it, I'm focused on the xitk xine-ui.

Hans-Dieter