[MPlayer-dev-eng] [PATCH] Mac OS X Finder support

[Reimar Döffinger]

Hi,
> >That is okay. Only problem with any kind of printf is that it will
> >hinder working support of piping to stdout (will this ever happen?).
> 
> No, because that part of the code is only called when MPlayer is  
> started from the finder. In these cases, stdin is already /dev/null and  
> both stdout and stderr are redirected to  
> /Library/Logs/Consoles/<username>/console.log

Yes, I understood that. I just wanted to explain why printf is usually
not allowed in MPlayer but in this case it is okay.

> >>+               Boolean valid;
> >MPlayer usually uses int (and 0/1 instead of FALSE/TRUE) because  
> >Boolean is not standard C. But that's up to you to decide how you want  
> >it.
> 
> That's the carbon Boolean. One could decide to use MPlayer's boolean if  
> there's one.

MPlayer's boolean is called int ;-). As I said, do whatever you want, I
just wanted to give you the choice.

> >>+                       argv[0]=NULL;
> >If you have a simple way to pass it something useful, do it.
> 
> The "real" argv[0] is going to be something like  
> /Applications/MPlayer.app/Contents/MacOS/mplayer which is not what the  
> user expects. The right thing to do is something along:

Does the user see it? I think he shouldn't. The sense of this parameter
is usually to enable the application to find out how it was called (e.g.
when compiled with X + GUI support, MPlayer uses it to find out whether
it was called as mplayer or gmplayer). As I think it is possible to
compile MPlayer under OSX with GUI I fear it might make it crash...

> {
> ProcessSerialNumber mypsn;
> CFStringRef name;
> char *procname;
> int len;
> 
> 	GetCurrentProcess(&mypsn);
> 	CopyProcessName(&mypsn,&name);
> 	len=CFStringGetLength(name)+1;
> 	argv[0]=malloc(len);
> 	CFStringGetCString(name,argv[0],len,kCFStringEncodingISOLatin1);
> 	argv[0][len-1]=0;
> }
> 
> I'm not sure if this can be called "simple".

No, that's overkill.

> >>+  
> >>snprintf(myPsnStr,5+10+1+10+1,"- 
> >>psn_%u_%u",myPsn.highLongOfPSN,myPsn.lowLongOfPSN);
> >call me paranoid, but I'd actually prefer something like
> >snprintf(myPsnStr, 5+10+1+10, ...);
> >myPsnStr[5+10+1+10] = 0;
> 
> There should be actually no chance for an unsigned long to be more than  
> 10 chars: '4294967296'...

As I said, I'm probably paranoid. But actually from the consideration of how
easily it can be broken by future changes I think it is the better
solution.

> Should I send a fourth version of the patch?

I would prefer that, it always looks stupid when the CVS log message
says "modified patch from ...".
And I forgot to say before: if you can, please set the
attachment type to text/plain instead of application/octet-stream, makes
it easier for me to read anid quote.

Greetings,
Reimar Döffinger

P.S.: Nicolas, if that version then works for you, feel free to apply...