headers
KO Myung-Hun | 1 Mar 2009 12:19

Re: [PATCH] vo_kva

Hi/2.

KO Myung-Hun wrote:
> Hi/2.
>
> Diego Biurrun wrote:
>   
>> On Thu, Feb 26, 2009 at 12:00:23AM +0900, KO Myung-Hun wrote:
>>   
>>     
>>> This patch adds a vo_kva for OS/2 video system.
>>>
>>> --- libvo/vo_kva.c	(revision 0)
>>> +++ libvo/vo_kva.c	(revision 0)
>>>  <at>  <at>  -0,0 +1,1194  <at>  <at> 
>>> +
>>> +typedef struct tagVOKVAINTERNAL {
>>> +} VOKVAINTERNAL, *PVOKVAINTERNAL;
>>>     
>>>       
>> typedefs are ugly..
>>
>>   
>>     
>
> Ok.
>
>   
>>> +extern void mplayer_put_key(int code);            // let mplayer handel the keyevents
>>>
(Continue reading)

Permalink | Reply | 22 comments |
headers
Diego Biurrun | 1 Mar 2009 12:39
Picon
Gravatar

Re: [PATCH] ao_dart

On Thu, Feb 26, 2009 at 09:12:37PM +0900, KO Myung-Hun wrote:
>
> Diego Biurrun wrote:
>> On Thu, Feb 26, 2009 at 05:02:09AM +0900, KO Myung-Hun wrote:
>>   
>>> [...]
>>>     
>>
>> Your patch fails to apply, so here is another round of nitpicks
>> instead of a commit...
>>
>>   
>>> --- libao2/ao_dart.c	(revision 0)
>>> +++ libao2/ao_dart.c	(revision 0)
>>>  <at>  <at>  -0,0 +1,332  <at>  <at> 
>>> +static int control(int cmd, void *arg)
>>> +{
>>> +    switch (cmd) {
>>> +        case AOCONTROL_GET_VOLUME :
>>
>> ...and the case at the same indentation depth as the switch.
>
> Fixed.

.. but only in one of two places ..

> But, now the closing bracket '}' of last case is placed on the same
> depth with the closing bracket '}' of switch. I think it's not good
> style.
(Continue reading)

Permalink | Reply | 5 comments |
headers
Diego Biurrun | 1 Mar 2009 12:50
Picon
Gravatar

Re: [PATCH] vo_kva

On Sun, Mar 01, 2009 at 08:19:35PM +0900, KO Myung-Hun wrote:
>
> KO Myung-Hun wrote:
>>
>> Diego Biurrun wrote:
>>   
>>> On Thu, Feb 26, 2009 at 12:00:23AM +0900, KO Myung-Hun wrote:
>>>       
>>>> This patch adds a vo_kva for OS/2 video system.
>>>>
>>>> --- libvo/vo_kva.c	(revision 0)
>>>> +++ libvo/vo_kva.c	(revision 0)
>>>>  <at>  <at>  -0,0 +1,1194  <at>  <at> 
>>>> +            //if (ustflags & (TF_LEFT | TF_RIGHT | TF_TOP | TF_BOTTOM | TF_SETPOINTERPOS))
>>>> +            {
>>>> +            #if 0
>>>> +                pti->rclBoundary.xLeft   = 0;
>>>> +                pti->rclBoundary.yBottom = 0;
>>>> +                pti->rclBoundary.xRight  = vo_screenwidth;
>>>> +                pti->rclBoundary.yTop    = vo_screenheight;
>>>> +            #endif
>>>>           
>>> What is this disabled code good for?  If it's good for nothing, get rid
>>> of it.
>>
>> It is for tracking boundary. If it's set, movie window does not go out  
>> of screen. If possible, I want to preserve it. If needed, I'll add  
>> comments for it.

At least add a comment.  But I disagree that it should be preserved.
(Continue reading)

Permalink | Reply | 21 comments |
headers
KO Myung-Hun | 1 Mar 2009 13:59

Re: [PATCH] ao_dart

Hi/2.

Diego Biurrun wrote:
>> But, now the closing bracket '}' of last case is placed on the same
>> depth with the closing bracket '}' of switch. I think it's not good
>> style.
>>     
>
> Just look at what K&R really does: Indent the opening brace after 'case'.
>
>   

How do I that in this case ?

    case 1234:
        {
        if (true) {
        }
        break;
        }

Just do that ?

>>>> +static int play(void *data, int len, int flags)
>>>> +{
>>>> +
>>>> +     if (!(flags & AOPLAY_FINAL_CHUNK))
>>>> +        len = (len / ao_data.outburst) * ao_data.outburst;
>>>> +
>>>> +     return write_buffer(data, len);
(Continue reading)

Permalink | Reply | 4 comments |
headers
KO Myung-Hun | 1 Mar 2009 14:04

Re: [PATCH] vo_kva

Hi/2.

Diego Biurrun wrote:
> On Sun, Mar 01, 2009 at 08:19:35PM +0900, KO Myung-Hun wrote:
>   
>> KO Myung-Hun wrote:
>>     
>>> Diego Biurrun wrote:
>>>   
>>>       
>>>> On Thu, Feb 26, 2009 at 12:00:23AM +0900, KO Myung-Hun wrote:
>>>>       
>>>>         
>>>>> This patch adds a vo_kva for OS/2 video system.
>>>>>
>>>>> --- libvo/vo_kva.c	(revision 0)
>>>>> +++ libvo/vo_kva.c	(revision 0)
>>>>>  <at>  <at>  -0,0 +1,1194  <at>  <at> 
>>>>> +            //if (ustflags & (TF_LEFT | TF_RIGHT | TF_TOP | TF_BOTTOM | TF_SETPOINTERPOS))
>>>>> +            {
>>>>> +            #if 0
>>>>> +                pti->rclBoundary.xLeft   = 0;
>>>>> +                pti->rclBoundary.yBottom = 0;
>>>>> +                pti->rclBoundary.xRight  = vo_screenwidth;
>>>>> +                pti->rclBoundary.yTop    = vo_screenheight;
>>>>> +            #endif
>>>>>           
>>>>>           
>>>> What is this disabled code good for?  If it's good for nothing, get rid
>>>> of it.
(Continue reading)

Permalink | Reply | 20 comments |
headers
Diego Biurrun | 1 Mar 2009 14:10
Picon
Gravatar

Re: [PATCH] ao_dart

On Sun, Mar 01, 2009 at 09:59:30PM +0900, KO Myung-Hun wrote:
> 
> Diego Biurrun wrote:
> >> But, now the closing bracket '}' of last case is placed on the same
> >> depth with the closing bracket '}' of switch. I think it's not good
> >> style.
> >
> > Just look at what K&R really does: Indent the opening brace after 'case'.
> 
> How do I that in this case ?
> 
>     case 1234:
>         {
>         if (true) {
>         }
>         break;
>         }
> 
> Just do that ?

Fine with me.

Diego
Permalink | Reply | 3 comments |
headers
Reimar Döffinger | 1 Mar 2009 14:13
Picon
Picon

Re: [PATCH] ao_dart

On Sun, Mar 01, 2009 at 02:10:58PM +0100, Diego Biurrun wrote:
> On Sun, Mar 01, 2009 at 09:59:30PM +0900, KO Myung-Hun wrote:
> > 
> > Diego Biurrun wrote:
> > >> But, now the closing bracket '}' of last case is placed on the same
> > >> depth with the closing bracket '}' of switch. I think it's not good
> > >> style.
> > >
> > > Just look at what K&R really does: Indent the opening brace after 'case'.
> > 
> > How do I that in this case ?
> > 
> >     case 1234:
> >         {
> >         if (true) {
> >         }
> >         break;
> >         }
> > 
> > Just do that ?
> 
> Fine with me.

And that's supposed to be not ugly?
Permalink | Reply | 2 comments |
headers
Diego Biurrun | 1 Mar 2009 14:18
Picon
Gravatar

Re: [PATCH] vo_kva

On Sun, Mar 01, 2009 at 10:04:21PM +0900, KO Myung-Hun wrote:
>
> Diego Biurrun wrote:
>> On Sun, Mar 01, 2009 at 08:19:35PM +0900, KO Myung-Hun wrote:
>>   
>>> KO Myung-Hun wrote:
>>>     
>>> --- DOCS/man/en/mplayer.1	(revision 28767)
>>> +++ DOCS/man/en/mplayer.1	(working copy)
>>>  <at>  <at>  -3573,6 +3585,22  <at>  <at> 
>>>  .
>>> +.IPs (no)t23
>>> +Enable/disable workaround for T23 laptop (default: \-not23).
>>
>> ?
>
> Added more detailed explanation.
>
> --- libvo/vo_kva.c	(revision 0)
> +++ libvo/vo_kva.c	(revision 0)
>  <at>  <at>  -0,0 +1,1193  <at>  <at> 
> +static void imgDisplay(void)
> +{
> +    PVOID       pBuffer;
> +    ULONG       ulBPL;

Any particular reason to use so many spaces?  Not that it matters..

> +#define MRETURN(ret)  return (MRESULT)(ret)
(Continue reading)

Permalink | Reply | 19 comments |
headers
Diego Biurrun | 1 Mar 2009 14:21
Picon
Gravatar

Re: [PATCH] ao_dart

On Sun, Mar 01, 2009 at 02:13:38PM +0100, Reimar Döffinger wrote:
> On Sun, Mar 01, 2009 at 02:10:58PM +0100, Diego Biurrun wrote:
> > On Sun, Mar 01, 2009 at 09:59:30PM +0900, KO Myung-Hun wrote:
> > > 
> > > Diego Biurrun wrote:
> > > >> But, now the closing bracket '}' of last case is placed on the same
> > > >> depth with the closing bracket '}' of switch. I think it's not good
> > > >> style.
> > > >
> > > > Just look at what K&R really does: Indent the opening brace after 'case'.
> > > 
> > > How do I that in this case ?
> > > 
> > >     case 1234:
> > >         {
> > >         if (true) {
> > >         }
> > >         break;
> > >         }
> > > 
> > > Just do that ?
> > 
> > Fine with me.
> 
> And that's supposed to be not ugly?

K&R as executed by the 'indent' program places the if at the next
indentation depth.  I don't particularly care.

Diego
(Continue reading)

Permalink | Reply | 0 comments |
headers
KO Myung-Hun | 1 Mar 2009 15:09

Re: [PATCH] vo_kva

Hi/2.

Diego Biurrun wrote:
>> --- libvo/vo_kva.c	(revision 0)
>> +++ libvo/vo_kva.c	(revision 0)
>>  <at>  <at>  -0,0 +1,1193  <at>  <at> 
>> +static void imgDisplay(void)
>> +{
>> +    PVOID       pBuffer;
>> +    ULONG       ulBPL;
>>     
>
> Any particular reason to use so many spaces?  Not that it matters..
>
>   

No. But maybe it's due to my habit of align variables. I think there 
were a variable with long name type and deleted it, then I didn't 
realign them.

Fixed, anyway.

>> +#define MRETURN(ret)  return (MRESULT)(ret)
>>     
>
> This just looks like obfuscation to me.
>
>   

I agree not, but changed.
(Continue reading)

Permalink | Reply | 18 comments |

Gmane