[MPlayer-dev-eng] [PATCH] Network synchronized playback using UDP

[Diego Biurrun]

On Fri, Apr 10, 2009 at 09:55:03AM -0700, Jason Holt wrote:
> 
> > Also, I wonder if this has to be in mplayer.c, which is already big
> > enough.
> 
> I'm happy to move it.  stream/udp.c is the most related source file I could
> find, but it's a little weird to have it in stream/.  I could create new
> .c/.h files if you think it's worthwhile.

You could create new source files, that sounds sensible.

> --- DOCS/xml/en/usage.xml	(revision 29167)
> +++ DOCS/xml/en/usage.xml	(working copy)
> @@ -494,6 +494,74 @@
> +
> +<para>
> +If <option>-udp-master</option> is given, <application>MPlayer</application>
> +sends a datagram to <option>-udp-ip</option> (default: 127.0.0.1)
> +on <option>-udp-port</option> (default: 23867) just before playing each frame.
> +The datagram indicates the master's position in the file. If 
> +<option>-udp-slave</option> is given, <application>MPlayer</application> listens on 
> +<option>-udp-ip</option>/<option>-udp-port</option>
> +and matches the master's position. Setting <option>-udp-ip</option> to the
> +master's broadcast address allows multiple slaves having the same broadcast
> +address to sync to the master.
> +</para>

This sounds as if -udp-ip and -udp-port are required, are they?

> +<para>
> +<option>-udp-seek-threshold</option> (default: 1) should probably be left
> +at its default setting. When the master seeks, the slave has to decide
> +whether to seek as well, or to catch up by decoding frames without pausing
> +between frames. If the master is more than
> +<option>-udp-seek-threshold</option> seconds away from the slave, the slave
> +seeks. Otherwise, it "runs" to catch up or waits for the master.  
> +</para>

Maybe this could be placed after the example since it is advanced usage.
But the description should not duplicate the content of the man page.
Just talk about possible use cases for the option.

> +<para>
> +On each slave, run:
> +</para>
> +
> +<screen>
> +mplayer -udp-slave videoN.mpg
> +</screen>

Why 'N'?

> --- DOCS/man/en/mplayer.1	(revision 29167)
> +++ DOCS/man/en/mplayer.1	(working copy)
> @@ -1149,8 +1149,36 @@
>  .
> +.TP
> +.B \-udp\-seek\-threshold <sec>
> +This should probably be left at its default setting of 1 second. 

That's a very vague statement and if at all, it should come after the
description of the option.

> --- mplayer.c	(revision 29167)
> +++ mplayer.c	(working copy)
> @@ -743,6 +766,163 @@
>  
> +    sockfd=socket(AF_INET,SOCK_DGRAM,0);

extra good karma for spaces around the '='.

> +    memset(&servaddr,sizeof(servaddr), 0);
> +    servaddr.sin_family = AF_INET;
> +    servaddr.sin_addr.s_addr=htonl(INADDR_ANY);
> +    servaddr.sin_port=htons(udp_port);

align

> +  if (blocking) {
> +    fcntl(sockfd, F_SETFL, sock_flags & (~O_NONBLOCK));
> +  } else {
> +    fcntl(sockfd, F_SETFL, sock_flags | O_NONBLOCK);
> +  }

pointless {}

> +  n = recvfrom(sockfd,mesg,999,0,(struct sockaddr *)&cliaddr,&len);

extra good karma for spaces around the ','.

> +    if ((my_position > udp_master_position - udp_seek_threshold) &&
> +        (my_position < udp_master_position + udp_timing_tolerance)) {
> +      closebehind = 1;
> +    } else {
> +      closebehind = 0;
> +    }

pointless {}

Diego