[vlc-devel] [PATCH 4/4] sout: add airplay support

[Felix Paul Kühne]

Hi Alexander,

This is great work! Some nit-pick inline.

> On 9. Oct 2018, at 00:47, Alexander Lyon via vlc-devel <vlc-devel at videolan.org> wrote:
> 
> diff --git a/modules/stream_out/airplay/airplay.c b/modules/stream_out/airplay/airplay.c
> new file mode 100644
> index 0000000000..b7db2160c5
> --- /dev/null
> +++ b/modules/stream_out/airplay/airplay.c
> @@ -0,0 +1,2771 @@
> +/*****************************************************************************
> + * airplay.c: Airplay v1 Streaming Support

This is AirPlay 2 only, no? If so, this should be written here and maybe the file should even be renamed in case we add support for AirPlay 1 aka RAOP, too.

> +#ifdef AIRPLAY_DIGEST

What is AirPlay Digest and why is it never enabled? This would be a lot of dead code or I’m missing something.

> +
> +    // get ipv4 address accessible by apple TV
> +    struct addrinfo *res = NULL;
> +    struct addrinfo hints = {
> +        .ai_family = AF_INET,
> +        .ai_socktype = SOCK_STREAM,
> +    };

The following code path ignores that a device could expose and announce another port as noted (and stored within the module as p_sys->i_port) by the bonjour discoverer.

> +    vlc_getaddrinfo( p_sys->psz_bonjour_hostname, 7000, &hints, &res );
> +
> +    char *psz_ipv4_ip = calloc( NI_MAXNUMERICHOST, sizeof( *psz_ipv4_ip ) );
> +    int port = 7000;
> +
> +    vlc_getnameinfo( res->ai_addr, res->ai_addrlen, psz_ipv4_ip, NI_MAXNUMERICHOST, &port, NI_NUMERICHOST );
> +    int ipv4_fd = net_ConnectTCP( p_this, psz_ipv4_ip, port );
> +    net_GetSockAddress( ipv4_fd, psz_ipv4_ip, &port );
> +    p_sys->psz_ipv4_ip = psz_ipv4_ip;
> +    net_Close( ipv4_fd );
> +

[snap]

> +
> +    msg_Dbg( p_this, "Sent %zd bytes to %s on port %d (status %"
> +        PRId16
> +        ")",
> +             bytes_written, p_sys->psz_bonjour_hostname, p_sys->i_port, i_status );
> +
> +    size_t i_resp_content_length = vlc_strtoi( vlc_dictionary_value_for_key( p_resp_headers, "Content-Length" ) );

Missing sanity check. vlc_dictionary_value_for_key may return NULL which will make vlc_strtoi crash.

Generally speaking, there are few compilation warnings left. Could you polish a bit so it compiles without?   

Best regards,

Felix