[MPlayer-dev-eng] [PATCH] '-priority' support on OS/2

[KO Myung-Hun]

Hi/2.

Diego Biurrun wrote:
> On Thu, Feb 05, 2009 at 10:46:21PM +0900, KO Myung-Hun wrote:
>   
>> Diego Biurrun wrote:
>>     
>>> On Thu, Feb 05, 2009 at 02:09:49AM +0900, KO Myung-Hun wrote:
>>>  
>>>       
>>>> I attach the updated patch.
>>>>
>>>> --- Makefile	(revision 28306)
>>>> +++ Makefile	(working copy)
>>>> @@ -433,6 +433,7 @@
>>>>
>>>> SRCS_COMMON-$(PNG) 				  += libmpcodecs/vd_mpng.c
>>>> +SRCS_COMMON-$(PRIORITY)				 += 
>>>> osdep/$(PRIORITY_C)
>>>> SRCS_COMMON-$(PVR) 				  += stream/stream_pvr.c
>>>> SRCS_COMMON-$(QTX_CODECS)			  += 
>>>> libmpcodecs/ad_qtaudio.c \
>>>>         
>>> Suddenly this file is full of tabs, what happened?
>>>       
>> It's make file, so I compressed spaces to tab as the original. It seems 
>> that tab is not matter, therefore I'll not convert spaces to tab.
>>     
>
> The original has tabs only in the required places.  As a rule of thumb,
> do not mess with whitespace.
>
>   

Hmm... See Makefile. There are many parts with a mixture of tabs and 
spaces, where tabs are not required.

Anyway, I understand your rule of thumb.

>> --- Makefile	(revision 28306)
>> +++ Makefile    (working copy)
>> @@ -215,6 +215,7 @@
>>                libvo/osd.c \
>>                libvo/sub.c \
>>                osdep/$(GETCH) \
>> +              osdep/priority.c \
>>     
>
> This should be compiled conditional on CONFIG_PRIORITY.
>
>   

Ok.

>> @@ -721,6 +725,7 @@
>>  _def_stream_cache="#define CONFIG_STREAM_CACHE 1"
>>  _def_pthread_cache="#undef PTHREAD_CACHE"
>>  _need_shmem=yes
>> +_def_priority="#undef CONFIG_PRIORITY"
>>     
>
> Leave out the leading underscore from the variable name 

I don't know what you mean. T.T

> and group all
> the definition variables together.
>
> Also, something is wrong with your patch, these hunks are for configure,
> not Makefile...
>
>   

Does this apply to _def_stream_cache and _def_pthread_cache ?

>> --- osdep/priority.c	(revision 0)
>> +++ osdep/priority.c	(revision 0)
>> @@ -0,0 +1,98 @@
>> +
>> +#include "config.h"
>> +
>> +#ifdef CONFIG_PRIORITY
>>     
>
> Hmmm...  I think it would be cleaner if you would do the call to
> set_priority in mplayer.c conditional to CONFIG_PRIORITY instead of
> using #ifdef here.
>
>   

Ok.

>> +    if(proc_priority) {
>> +
>> +        for(i = 0; priority_presets_defs[i].name; i++) {
>> +            if(strcasecmp(priority_presets_defs[i].name, proc_priority) == 0)
>>     
>
> Please use K&R style, i.e. space after if/for.
>
>   

Fixed.

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 1.1.14
Under OS/2 Warp 4 for Korean with FixPak #15
On AMD ThunderBird 1 GHz with 512 MB RAM

Korean OS/2 User Community : http://www.ecomstation.co.kr


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: priority.diff
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090206/83a09fff/attachment.asc>