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

[compn]

On Mon, 09 Mar 2009 20:26:47 +0100, Francesco Cosoleto wrote:

i didnt test patch, this is just a non-programmer review.

>-#include "url.h"
>+#include "libavutil/intreadwrite.h"
> #include "libmpdemux/asf.h"
>+#include "libmpdemux/asfguid.h"
> 
>+#include "url.h"
> #include "stream.h"
>-

cosmetics

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

is the //fixme needed?
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"

>-#if 0
>-      int b = i;
>-      printf ("unknown object (guid: %016llx, %016llx, len: %lld)\n", guid_1, guid_2, length);
>-      for (; b < length; b++)
>-      {
>-        if (isascii(header[b]) || isalpha(header[b]))
>-	    printf("%c ", header[b]);
>-	else
>-    	    printf("%x ", header[b]);
>-      }
>-      printf("\n");
>-#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.

>+  //FIXME error messages non localized

i dont think its needed...

>+              //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?

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...

-compn