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

[Diego Biurrun]

On Tue, Apr 07, 2009 at 11:13:16AM -0700, Jason Holt wrote:
> I added xml/manpage docs and configure script autodetection.  Anything else
> I missed?  (Let me know if I should have done the configure script
> differently).
> 
> I also fixed a little bug in the configure script example at the top.

Such things need to be different patches.  Never mind now, I extracted
and applied it.

> --- AUTHORS	(revision 29150)
> +++ AUTHORS	(working copy)
> @@ -368,6 +368,9 @@
>  
> +Holt, Jason <jholt [at] google.com>
> +    * udp network sync

UDP network synchronization

> --- configure	(revision 29150)
> +++ configure	(working copy)
> @@ -3059,7 +3064,6 @@
>  fi
>  echores "$_network"
>  
> -
>  echocheck "inet6"

cosmetics

> @@ -3967,7 +3971,31 @@
>  
> +echocheck "network sync prerequisites"
> +if test "$_network_sync" = auto; then
> +cat > $TMPC << EOF
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <strings.h>
> +#include <netdb.h>
> +int main(void) { return 0; }
> +EOF
> +fi
> +_network_sync=no
> +cc_check && _network_sync=yes
> +if test "$_network_sync" = yes ; then
> +  def_network_sync='#define CONFIG_NETWORK_SYNC 1'
> +else
> +  def_network_sync='#undef CONFIG_NETWORK_SYNC'
> +fi
> +echores "$_network_sync"

What are you testing there?  All of these look like standard headers
that we use without check in other places.

> --- DOCS/xml/en/usage.xml	(revision 29150)
> +++ DOCS/xml/en/usage.xml	(working copy)
> @@ -494,9 +494,74 @@
> +
> +<!-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -->
> +
> +<sect1 id="networksync" xreflabel="Network Synchronized Playback">
> +<title>Synchronized playback over a network</title>
> +
> +<para>
> +Multiple instances of <application>MPlayer</application> can synchronize
> +playback over a network.  This is useful for creating "video walls" with
> +multiple screens controlled by different computers.  Each mplayer instance can
> +play a different video, but they all will try to stay at the same time offset
> +in the file.  It is recommended but not necessary to encode the video files
> +using the same codec and parameters.
> +</para>

Use just a single space after a period and write MPlayer consistently.

> --- DOCS/man/en/mplayer.1	(revision 29150)
> +++ DOCS/man/en/mplayer.1	(working copy)
> @@ -1149,6 +1149,28 @@
>  The normal framerate of the movie is kept, so playback is accelerated.
>  Since MPlayer can only seek to the next keyframe this may be inexact.
>  .
> +.TP
> +.B \-udp-master
> +.TP
> +.B \-udp-slave
> +.TP
> +.B \-udp-ip <ip>
> +.TP
> +.B \-udp-port <port>
> +.TP
> +.B \-udp-seek-threshold <sec>
> +These options allow multiple instances of mplayer to synchronize playback over
> +a network.  If -udp-master is given, mplayer sends a datagram to -udp-ip (default:
> +127.0.0.1) on -udp-port (default: 23867) just before playing each frame.  The
> +datagram indicates the master's position in the file.  If -udp-slave is given,
> +mplayer listens on -udp-port and matches the master's position.
> +Setting -udp-ip to a broadcast address allows multiple slaves having the same
> +broadcast address to sync to the master.
> +-udp-seek-threshold (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 -udp-seek-threshold seconds away from the slave, the slave
> +seeks.  Otherwise, it "runs" to catch up or waits for the master.  

Explain each option separately, not all in one block.  Start new
sentences on a new line in the man page.

> --- mplayer.c	(revision 29150)
> +++ mplayer.c	(working copy)
> @@ -357,6 +370,20 @@
>  
> +#ifdef CONFIG_NETWORK_SYNC
> +// config options for UDP sync
> +int udp_master = 0; // are we a udp master?
> +int udp_slave = 0;  // are we a udp slave?

redundant comments

> @@ -679,8 +706,105 @@
>  
> +#ifdef CONFIG_NETWORK_SYNC
> +void get_udp(int blocking) {

Please use K&R style for new code.

> --- cfg-mplayer.h	(revision 29150)
> +++ cfg-mplayer.h	(working copy)
> @@ -288,6 +288,14 @@
>  
> +#ifdef CONFIG_NETWORK_SYNC
> +	{"udp-slave", &udp_slave, CONF_TYPE_FLAG, 0, 0, 1, NULL},
> +	{"udp-master", &udp_master, CONF_TYPE_FLAG, 0, 0, 1, NULL},
> +	{"udp-ip", &udp_ip, CONF_TYPE_STRING, 0, 0, 1, NULL},
> +	{"udp-port", &udp_port, CONF_TYPE_INT, 0, 1, 65535, NULL},
> +        {"udp-seek-threshold", &udp_seek_threshold, CONF_TYPE_FLOAT, CONF_RANGE, 0.1, 100, NULL},
> +#endif

tabs

Diego