[vlc-devel] [PATCHv4 1/3] access: Add satip access module

Julian Scheel julian at jusst.de
Fri May 27 11:43:26 CEST 2016


Thanks for the review.

On 27.05.2016 10:49, Rémi Denis-Courmont wrote:
> Le 2016-05-26 16:51, Julian Scheel a écrit :
>> +static int check_rtp_seq(access_t *access, block_t *block)
>> +{
>> +    access_sys_t *sys = access->p_sys;
>> +    uint16_t seq_nr = block->p_buffer[2] << 8 | block->p_buffer[3];
>> +
>> +    if (seq_nr == sys->last_seq_nr) {
>> +        msg_Warn(access, "Received duplicate packet (seq_nr=%u)",
>> seq_nr);
>> +        return VLC_EGENERIC;
>> +    } else if (seq_nr < ++sys->last_seq_nr) {
>
> So what if last_seq_nr was 65535 before the increment?

last_seq_nr will be wrapped to 0, just as seq_nr, so no warning will occur.

>> +        msg_Warn(access, "Received out of order packet (seq_nr=%u <
>> %u)",
>> +                seq_nr, sys->last_seq_nr);
>
> PRIu16, same below.

Ack.

>> +        return VLC_EGENERIC;
>> +    } else if (sys->last_seq_nr > 1 && seq_nr > sys->last_seq_nr) {
>> +        msg_Warn(access, "Gap in seq_nr (%u > %u), probably lost a
>> packet",
>> +                seq_nr, sys->last_seq_nr);
>> +    }
>> +
>> +    sys->last_seq_nr = seq_nr;
>> +    return 0;
>> +}
>> +
>> +static void satip_teardown(void *data) {
>> +    access_t *access = data;
>> +    access_sys_t *sys = access->p_sys;
>> +
>> +    int canc = vlc_savecancel();
>
> You might be better off disabling cancellation

documentation of vlc_savecancel
vlc_thread.h:645: "Disables thread cancellation."

>> +    if (sys->tcp_sock > 0) {
>> +        if (sys->session_id[0] > 0) {
>> +            net_Printf(access, sys->tcp_sock,
>> +                    "TEARDOWN %s RTSP/1.0\r\n"
>> +                    "CSeq: %d\r\n"
>> +                    "Session: %s\r\n\r\n",
>> +                    sys->control, sys->cseq++, sys->session_id);
>> +            if (rtsp_handle(access) != RTSP_RESULT_OK)
>> +                msg_Warn(access, "Failed to teardown RTSP session");
>> +        }
>> +    }
>> +    vlc_restorecancel(canc);
>
> Won't this lock up if the connection remains congested for writing?
> (ditto other use of net_Printf())

Hm, I don't see why that would ever happen.
Assuming it enters a state of write congestion would you mind suggesting 
some approach to properly deal with it?

>> +}
>> +
>> +static void *satip_thread(void *data) {
>> +    access_t *access = data;
>> +    access_sys_t *sys = access->p_sys;
>> +    int sock = sys->udp_sock;
>> +    volatile int sleeps = 0;
>> +    block_t *block = NULL;
>> +    ssize_t len;
>> +    mtime_t next_keepalive = mdate() + sys->keepalive_interval *
>> 1000 * 1000;
>> +
>> +#ifdef HAVE_RECVMMSG
>> +    struct mmsghdr msgs[VLEN];
>> +    struct iovec iovecs[VLEN];
>> +    int retval, i;
>> +    memset(msgs, 0, sizeof(msgs));
>> +#endif
>> +
>> +    vlc_cleanup_push(satip_teardown, access);
>> +    while (sleeps < 100) {
>> +#ifdef HAVE_RECVMMSG
>> +        if (alloc_input_blocks(access) != 0) {
>> +            msg_Err(access, "Failed to allocate memory for input
>> buffers");
>> +            break;
>> +        }
>> +
>> +        for(i = 0; i < VLEN; ++i) {
>> +            iovecs[i].iov_base = sys->input_blocks[i]->p_buffer;
>> +            iovecs[i].iov_len = RTSP_RECEIVE_BUFFER;
>> +            msgs[i].msg_hdr.msg_iov = &iovecs[i];
>> +            msgs[i].msg_hdr.msg_iovlen = 1;
>> +        }
>> +        retval = recvmmsg(sock, msgs, VLEN, MSG_WAITFORONE, NULL);
>> +        if (retval == -1) {
>> +            usleep(20000);
>
> No obsolete/removed function thanks.

Ack, changed to msleep.

>> +            ++sleeps;
>> +            continue;
>> +        }
>> +
>> +        sleeps = 0;
>> +        for(i = 0; i < retval; ++i) {
>> +            len = msgs[i].msg_len;
>> +            block = sys->input_blocks[i];
>> +            if (check_rtp_seq(access, block))
>> +                continue;
>> +
>> +            block->p_buffer += RTP_HEADER_SIZE;
>> +            block->i_buffer = len - RTP_HEADER_SIZE;
>> +            block_FifoPut(sys->fifo, block);
>> +            sys->input_blocks[i] = NULL;
>> +        }
>> +#else
>> +        struct pollfd ufd;
>> +
>> +        ufd.fd = sock;
>> +        ufd.events = POLLIN;
>> +        if (poll(&ufd, 1, 20) == -1) {
>
> No. Compute your deadline correctly.

Ack, dropped sleeps and am using mdate() - RECV_TIMEOUT now,

>> +            ++sleeps;
>> +            continue;
>> +        }
>> +
>> +        block = block_Alloc(RTSP_RECEIVE_BUFFER);
>> +        if (block == NULL) {
>> +            msg_Err(access, "Failed to allocate memory for input
>> buffer");
>> +            break;
>> +        }
>> +
>> +        len = recv(sock, block->p_buffer, RTSP_RECEIVE_BUFFER, 0);
>> +        if (len < RTP_HEADER_SIZE) {
>> +            block_Release(block);
>> +            continue;
>> +        }
>> +
>> +        if (check_rtp_seq(access, block)) {
>> +            block_Release(block);
>> +            continue;
>> +        }
>> +        sleeps = 0;
>> +        block->p_buffer += RTP_HEADER_SIZE;
>> +        block->i_buffer = len - RTP_HEADER_SIZE;
>> +        block_FifoPut(sys->fifo, block);
>> +#endif
>> +
>> +        if (sys->keepalive_interval > 0 && mdate() > next_keepalive) {
>> +            net_Printf(access, sys->tcp_sock,
>> +                    "OPTIONS %s RTSP/1.0\r\n"
>> +                    "CSeq: %d\r\n"
>> +                    "Session: %s\r\n\r\n",
>> +                    sys->control, sys->cseq++, sys->session_id);
>> +            if (rtsp_handle(access) != RTSP_RESULT_OK)
>> +                msg_Warn(access, "Failed to keepalive RTSP session");
>> +
>> +            next_keepalive = mdate() + sys->keepalive_interval *
>> 1000 * 1000;
>> +        }
>> +    }
>> +    vlc_cleanup_pop();
>> +    satip_teardown(access);
>> +
>> +    msg_Dbg(access, "timed out waiting for data...");
>> +    vlc_fifo_Lock(sys->fifo);
>> +    sys->woken = true;
>> +    vlc_fifo_Signal(sys->fifo);
>> +    vlc_fifo_Unlock(sys->fifo);
>> +
>> +    return NULL;
>> +}
>> +
>> +static block_t* satip_block(access_t *access) {
>> +    access_sys_t *sys = access->p_sys;
>> +    block_t *block;
>> +
>> +    vlc_fifo_Lock(sys->fifo);
>> +
>> +    while (vlc_fifo_IsEmpty(sys->fifo)) {
>> +        if (sys->woken)
>> +            break;
>> +        vlc_fifo_Wait(sys->fifo);
>> +        /* Make sure there is no cancellation point other than this
>> one^^.
>> +         * If you need one, be sure to push cleanup of p_block. */
>> +    }
>> +
>> +    int canc = vlc_savecancel();
>> +    block = vlc_fifo_DequeueUnlocked(sys->fifo);
>> +    sys->woken = false;
>> +    vlc_fifo_Unlock(sys->fifo);
>> +
>> +    vlc_restorecancel(canc);
>> +    return block;
>> +}
>> +
>> +static int satip_control(access_t *access, int i_query, va_list args) {
>> +    bool *pb_bool;
>> +    int64_t *pi_64;
>> +
>> +    switch(i_query)
>> +    {
>> +        case ACCESS_CAN_CONTROL_PACE:
>> +        case ACCESS_CAN_SEEK:
>> +            pb_bool = (bool*)va_arg(args, bool*);
>> +            *pb_bool = false;
>> +            break;
>> +
>> +        case ACCESS_CAN_PAUSE:
>> +            pb_bool = (bool*)va_arg(args, bool*);
>> +            *pb_bool = false;
>> +            break;
>> +
>> +        case ACCESS_GET_PTS_DELAY:
>> +            pi_64 = (int64_t*)va_arg(args, int64_t *);
>> +            *pi_64 = INT64_C(1000) * var_InheritInteger(access,
>> "live-caching");
>> +            break;
>> +
>> +        default:
>> +            return VLC_EGENERIC;
>> +
>> +    }
>> +    return VLC_SUCCESS;
>> +}
>> +
>> +/* Bind two adjacent free ports, of which the first one is even (for
>> RTP data)
>> + * and the second is odd (RTCP). This is a requirement of the satip
>> + * specification */
>> +static int satip_bind_ports(access_t *access)
>> +{
>> +    access_sys_t *sys = access->p_sys;
>> +    uint8_t rnd;
>> +
>> +    vlc_rand_bytes(&rnd, 1);
>> +    sys->udp_port = 9000 + (rnd * 2); /* randomly chosen, even start
>> point */
>> +    while (sys->udp_sock < 0 && sys->udp_port < 65535) {
>> +        sys->udp_sock = net_OpenDgram(access, "0.0.0.0",
>> sys->udp_port, NULL,
>> +                0, IPPROTO_UDP);
>> +        if (sys->udp_sock < 0) {
>> +            sys->udp_port += 2;
>> +            continue;
>> +        }
>> +
>> +        sys->rtcp_sock = net_OpenDgram(access, "0.0.0.0",
>> sys->udp_port + 1, NULL,
>> +                0, IPPROTO_UDP);
>> +        if (sys->rtcp_sock < 0) {
>> +            close(sys->udp_sock);
>> +            sys->udp_port += 2;
>> +            continue;
>> +        }
>> +    }
>> +
>> +    if (sys->udp_sock < 0) {
>> +        msg_Err(access, "Could not open two adjacent ports for RTP
>> and RTCP data");
>> +        return VLC_EGENERIC;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int satip_open(access_t *access) {
>> +    access_sys_t *sys;
>> +    vlc_url_t url;
>> +
>> +    bool multicast = var_InheritBool(access, "satip-multicast");
>> +
>> +    access->p_sys = sys = calloc(1, sizeof(*sys));
>> +    if (sys == NULL)
>> +        return VLC_ENOMEM;
>> +
>> +    msg_Dbg(access, "try to open '%s'", access->psz_location);
>> +
>> +    sys->udp_sock = -1;
>> +    sys->rtcp_sock = -1;
>> +
>> +    /* convert url to lowercase, some famous m3u playlists for satip
>> contain
>> +     * uppercase parameters while most (all?) satip servers do only
>> understand
>> +     * parameters matching lowercase spelling as defined in the
>> specification
>> +     * */
>> +    char *psz_lower_location = strdup(access->psz_location);
>> +    if (psz_lower_location == NULL)
>> +        goto error;
>> +
>> +    for (unsigned i = 0; i < strlen(psz_lower_location); i++)
>> +        psz_lower_location[i] = tolower(psz_lower_location[i]);
>> +
>> +    vlc_UrlParse(&url, psz_lower_location);
>> +    if (url.i_port <= 0)
>> +        url.i_port = RTSP_DEFAULT_PORT;
>> +
>> +    msg_Dbg(access, "connect to host '%s'", url.psz_host);
>> +    sys->tcp_sock = net_ConnectTCP(access, url.psz_host, url.i_port);
>> +    if (sys->tcp_sock < 0) {
>> +        msg_Err(access, "Failed to connect to RTSP server %s:%d",
>> +                url.psz_host, url.i_port);
>> +        goto error;
>> +    }
>> +    setsockopt (sys->tcp_sock, SOL_SOCKET, SO_KEEPALIVE, &(int){ 1
>> }, sizeof (int));
>
> This is useless if you do application layer keep-alives.

Ack, dropped.

>> +
>> +    if (asprintf(&sys->content_base, "rtsp://%s:%d/", url.psz_host,
>> +             url.i_port) < 0) {
>> +        sys->content_base = NULL;
>> +        goto error;
>> +    }
>> +
>> +    sys->last_seq_nr = 0;
>> +    sys->keepalive_interval = (KEEPALIVE_INTERVAL - KEEPALIVE_MARGIN);
>> +
>> +    if (multicast) {
>> +        net_Printf(access, sys->tcp_sock,
>> +                "SETUP rtsp://%s RTSP/1.0\r\n"
>> +                "CSeq: %d\r\n"
>> +                "Transport: RTP/AVP;multicast\r\n\r\n",
>> +                psz_lower_location, sys->cseq++);
>> +    } else {
>> +        /* open UDP socket to acquire a free port to use */
>> +        if (satip_bind_ports(access))
>> +            goto error;
>> +
>> +        net_Printf(access, sys->tcp_sock,
>> +                "SETUP rtsp://%s RTSP/1.0\r\n"
>> +                "CSeq: %d\r\n"
>> +                "Transport: RTP/AVP;unicast;client_port=%d-%d\r\n\r\n",
>> +                psz_lower_location, sys->cseq++, sys->udp_port,
>> sys->udp_port + 1);
>> +    }
>> +
>> +    if (rtsp_handle(access) != RTSP_RESULT_OK) {
>> +        msg_Err(access, "Failed to setup RTSP session");
>> +        goto error;
>> +    }
>> +
>> +    if (asprintf(&sys->control, "%sstream=%d", sys->content_base,
>> sys->stream_id) < 0) {
>> +        sys->control = NULL;
>> +        goto error;
>> +    }
>> +
>> +    /* Open UDP socket for reading if not done */
>> +    if (multicast) {
>> +        sys->udp_sock = net_OpenDgram(access, sys->udp_address,
>> sys->udp_port, "", sys->udp_port, IPPROTO_UDP);
>> +        if (sys->udp_sock < 0) {
>> +            msg_Err(access, "Failed to open UDP socket for listening.");
>> +            goto error;
>> +        }
>> +
>> +        sys->rtcp_sock = net_OpenDgram(access, sys->udp_address,
>> sys->udp_port + 1, "", sys->udp_port + 1, IPPROTO_UDP);
>> +        if (sys->rtcp_sock < 0) {
>> +            msg_Err(access, "Failed to open RTCP socket for
>> listening.");
>> +            goto error;
>> +        }
>> +    }
>> +    net_SetCSCov(sys->udp_sock, -1, RTP_HEADER_SIZE);
>> +
>> +    net_Printf(access, sys->tcp_sock,
>> +            "PLAY %s RTSP/1.0\r\n"
>> +            "CSeq: %d\r\n"
>> +            "Session: %s\r\n\r\n",
>> +            sys->control, sys->cseq++, sys->session_id);
>> +
>> +    if (rtsp_handle(access) != RTSP_RESULT_OK) {
>> +        msg_Err(access, "Failed to play RTSP session");
>> +        goto error;
>> +    }
>> +
>> +    sys->fifo = block_FifoNew();
>> +    if (!sys->fifo) {
>> +        msg_Err(access, "Failed to allocate block fifo.");
>> +        goto error;
>> +    }
>> +    sys->fifo_size = var_InheritInteger(access, "satip-buffer");
>> +
>> +    if (vlc_clone(&sys->thread, satip_thread, access,
>> VLC_THREAD_PRIORITY_INPUT)) {
>> +        msg_Err(access, "Failed to create worker thread.");
>> +        goto error;
>> +    }
>> +
>> +    access->pf_control = satip_control;
>> +    access->pf_block = satip_block;
>> +
>> +    free(psz_lower_location);
>> +    vlc_UrlClean(&url);
>> +    return VLC_SUCCESS;
>> +
>> +error:
>> +    if (psz_lower_location)
>> +        free(psz_lower_location);
>> +
>> +    vlc_UrlClean(&url);
>> +    satip_close(access);
>> +    return VLC_EGENERIC;
>> +}
>> +
>> +static void satip_close(access_t *access) {
>> +    access_sys_t *sys = access->p_sys;
>> +
>> +    if (sys->thread) {
>> +        vlc_cancel(sys->thread);
>> +        vlc_join(sys->thread, NULL);
>> +    }
>> +
>> +    if (sys->fifo)
>> +        block_FifoRelease(sys->fifo);
>> +
>> +#ifdef HAVE_RECVMMSG
>> +    for (int i = 0; i < VLEN; i++) {
>> +        if (sys->input_blocks[i])
>> +            block_Release(sys->input_blocks[i]);
>> +    }
>> +#endif
>> +
>> +    if (sys->udp_sock > 0)
>> +        net_Close(sys->udp_sock);
>> +    if (sys->rtcp_sock > 0)
>> +        net_Close(sys->rtcp_sock);
>> +    if (sys->tcp_sock > 0)
>> +        net_Close(sys->tcp_sock);
>> +
>> +    free(sys->content_base);
>> +    free(sys->control);
>> +    free(sys);
>> +}
>> diff --git a/po/POTFILES.in b/po/POTFILES.in
>> index 6451cc2..758fc9b 100644
>> --- a/po/POTFILES.in
>> +++ b/po/POTFILES.in
>> @@ -248,6 +248,7 @@ modules/access/rtsp/real_sdpplin.c
>>  modules/access/rtsp/real_sdpplin.h
>>  modules/access/rtsp/rtsp.c
>>  modules/access/rtsp/rtsp.h
>> +modules/access/satip.c
>>  modules/access/screen/mac.c
>>  modules/access/screen/screen.c
>>  modules/access/screen/screen.h
>



More information about the vlc-devel mailing list