[MPlayer-dev-eng] [PATCH] Improved MMS over TCP support

[Francesco Cosoleto]

compn wrote:
> i didnt test patch, this is just a non-programmer review.

Thank you.

>> -#include "url.h"
[...]
>> +#include "url.h"
>> #include "stream.h"
>> -

> cosmetics

Fixed.

>> +        //FIXME error messages non localized
>> +        mp_msg(MSGT_NETWORK, MSGL_FATAL, "Mandatory object in ASF header not found\n");
> 
> is the //fixme needed?

No, it isn't. All removed.

> also you could change "mandatory object" to whatever the real name of
> the not found object was. e.g. "barfoo in ASF header not found, exiting"

Replaced with: "File properties object in ASF header not found, exiting."

>> -#if 0
>> -      int b = i;
>> -      printf ("unknown object (guid: %016llx, %016llx, len: %lld)\n", guid_1, guid_2, length);
[...]
>> -#else
>> -      mp_msg(MSGT_NETWORK,MSGL_WARN,MSGTR_MPDEMUX_MMST_UnknownObject);
>> -#endif

> if #0 code should probably be removed seperately from this patch.
> in a cleanup/cosmetic patch i think.

That is part of a rewritten function, non compatible with the new code
that looks for known objects only. So I think it should die non seperately.

  >> +              //FIXME info message - audio/video stream selected
>> +              mp_msg(MSGT_NETWORK,MSGL_INFO,"Stream %d selected\n", i);
> 
> i shouldnt be asking about source comments, mplayer needs more.
> but what exactly is this one saying? just a copy of the mp_msg?

The type of stream, video or audio. Not so important, fixable later.

> we are short on devels and roberto (mms maintainer) hasnt been around in
> a while. 
> 
> would you be interested in maintaining this code? as long as other devs
> dont have any objections...

That looks as very premature. Isn't it? Encouraging though. I hope the
number of active developers can change in the future, as the project
need them.

-- 
Francesco Cosoleto | http://cosoleto.free.fr

Il figlio di Pausania, al sentir definire i Lacedemoni
ignoranti da un oratore ateniese, esclamò: «Ben detto!
infatti noi soli tra gli Elleni non abbiamo imparato
nessuna disonestà da voi» (Plutarco, Vita di Licurgo, 20)