[vlc-devel] [PATCH] Add Apple AirTunes stream output plugin

[Rémi Denis-Courmont]

	Hello,

A few comments...

Le jeudi 11 décembre 2008 02:46:48 Michael Hanselmann, vous avez écrit :
> Apple's AirPort Express can play audio streamed over the network. The
> protocol used was reverse-engineered by Jon Lech Johansen in 2004. He
> also released a proof of concept implementation named JustePort.
>
> A variant of HTTP is used to negotiate details before sending music
> data over an AES encrypted TCP connection. RSA is used to encrypt the
> AES key before transfering it via an HTTP header. To this day, only
> the public part of the RSA key has been made public while the private
> part remains unknown.

This "variant of HTTP" is called interleaved RTSP record mode :)

> The "airtunes" plugin can be used like this:
> --sout='#transcode{acodec=alac,channels=2}:airtunes{host=hostname}'

Is AirTunes a trademark or anything?

> +AC_ARG_ENABLE(airtunes,
> +  [  --enable-airtunes       AirTunes stream plugin (default enabled)])
> +if test "${enable_airtunes}" != "no"; then
> +  AS_IF([test "${have_libgcrypt}" = "yes"], [
> +    VLC_ADD_PLUGIN([stream_out_airtunes])
> +    VLC_ADD_LIBS([stream_out_airtunes], [${LIBGCRYPT_LIBS}])
> +    VLC_ADD_CFLAGS([stream_out_airtunes], [${LIBGCRYPT_CFLAGS}])
> +  ], [
> +    AC_MSG_WARN([libgcrypt support required for AirTunes plugin])
> +  ])
> +fi

Hmm. Do we really need yet another configure option? It seems to only depend 
on libgcrypt.

> +static const char psz_airtunes_rsa_pubkey[] =
> +    "e7d744f2a2e2788b6c1f55a08eb70544a8fa7945aa8be6c62ce5f51cbdd4dc68"
> +    "42fe3d1083dd2edec1bfd4252dc02e6f398bdf0e6148ea84855e2e442da6d626"
> +    "64f674a1f304929ade4f6893ef2df6e711a8c77a0d91c9d980822e50d12922af"
> +    "ea40ea9f0e14c0f76938c5f3882fc0323dd9fe55155f51bb5921c201629fd733"
> +    "52d5e2efaabf9ba048d7b813a2b6767f6c3ccf1eb4ce673d037b0d2ea30c5fff"
> +    "eb06f8d08adde409571a9c689fef10728855dd8cfb9a8bef5c8943ef3b5faa15"
> +    "dde698beddf3599603eb3e6f61372bb628f6559f599a78bf500687aa7f4976c0"
> +    "562d412956f8989e18a6355bd81597825e0fc875343ec782117625cdbf98447b";

Does MPI accept binary data? That would take half as much space in the binary. 
Not a big deal though.

> +static int _CheckForGcryptError( sout_stream_t *p_stream,
> +                                 gcry_error_t i_gcrypt_err,
> +                                 unsigned int i_line )

I know VLC is abusing them quite much already, but symbols starting with 
underscore are supposed to be reserved for the C run-time.

> +    /* Build SDP */
> +    i_rc = asprintf( &psz_sdp,
> +                     "v=0\r\n"
> +                     "o=iTunes %u 0 IN IP4 %s\r\n"
> +                     "s=iTunes\r\n"
> +                     "c=IN IP4 %s\r\n"
> +                     "t=0 0\r\n"
> +                     "m=audio 0 RTP/AVP 96\r\n"
> +                     "a=rtpmap:96 AppleLossless\r\n"
> +                     "a=fmtp:96 4096 0 16 40 10 14 2 255 0 0 44100\r\n"
> +                     "a=rsaaeskey:%s\r\n"
> +                     "a=aesiv:%s\r\n",
> +                     i_session_id, psz_local, p_sys->psz_host,
> +                     psz_aes_key_base64, psz_aes_iv_base64 );

I wonder if IP6 would ever make any sense here?
I also guess this is not really RTP but we cannot fix that :(

> +        /* Fill buffer */
> +        memcpy( p_sys->p_sendbuf, header, sizeof( header ) );
> +        memcpy( p_sys->p_sendbuf + sizeof( header ),
> +                p_buffer->p_buffer, p_buffer->i_buffer );

What about using block_Realloc() instead?

> +    var_AddCallback( p_stream, SOUT_CFG_PREFIX "volume",
> +                     VolumeCallback, NULL );

Currently VolumeCallback does nothing, so that's OK. But after that, you need 
to ensure proper thread synchronization, and call var_DelCallback() early 
enough in Close().

> +    default:
> +        /* Leave other stream types alone */
> +        break;

Perhaps, you need to check there is no more than one lossless ES at a time.

-- 
Rémi Denis-Courmont
http://git.remlab.net/cgi-bin/gitweb.cgi?p=vlc-courmisch.git;a=summary