[vlc-devel] [PATCH] Input and Output modules for the Reliable Internet Stream Transport (RIST) Protocol.

Jean-Baptiste Kempf jb at videolan.org
Sat Oct 20 01:17:22 CEST 2018


Hello,


+typedef struct
+{
+    struct rist_flow *f;

Rename this variable.

+    char             sender_name[MAX_CNAME];
+    enum NACK_TYPE   nack_type;
+    uint64_t         last_data_rx;
+    uint64_t         last_nack_tx;
+    vlc_thread_t     thread;
+    int              i_max_packet_size;
+    int              i_poll_timeout;
+    bool             eof_on_reset;
+    block_fifo_t     *p_fifo;
+    vlc_mutex_t      lock;
+} stream_sys_t;

Do you _need_ all that in this structure?

+static void send_rtcp_feedback(stream_t *p_access, struct rist_flow *f)
+{
+    stream_sys_t *p_sys = p_access->p_sys;
+    int namelen = strlen(f->cname) + 1;
+
+    int rtcp_feedback_size = RTCP_EMPTY_RR_SIZE + RTCP_SDES_SIZE + namelen;
+    block_t *pkt = block_Alloc( rtcp_feedback_size );
+    if ( unlikely( pkt == NULL ) )
+    {
+        return;
+    }
+
+    /* Populate RR */
+    uint8_t *rr = pkt->p_buffer;
+    rtp_set_hdr(rr);
+    rtcp_rr_set_pt(rr);
+    rtcp_set_length(rr, 1);
+    rtcp_fb_set_int_ssrc_pkt_sender(rr, 0);
+    /* Populate SDES */

Add vertical space before this.
This is true for most of your code, btw.

+    uint8_t *p_sdes = (pkt->p_buffer + RTCP_EMPTY_RR_SIZE);
+    if ((namelen - 2) & 0x3)
+        namelen = ((((namelen - 2) >> 2) + 1) << 2) + 2;
+    rtp_set_hdr(p_sdes);
+    rtp_set_cc(p_sdes, 1); /* Actually it is source count in this case */
+    rtcp_sdes_set_pt(p_sdes);
+    rtcp_set_length(p_sdes, (namelen >> 2) + 2);
+    rtcp_sdes_set_cname(p_sdes, 1);
+    rtcp_sdes_set_name_length(p_sdes, strlen(f->cname));
+    uint8_t *p_sdes_name = (pkt->p_buffer + RTCP_EMPTY_RR_SIZE + RTCP_SDES_SIZE);
+    strcpy((char *)p_sdes_name, f->cname);
+    vlc_mutex_lock( &p_sys->lock );
+    rist_WriteTo(f->fd_nack, pkt->p_buffer, rtcp_feedback_size, (struct sockaddr *)&f->peer_sockaddr, f->peer_socklen);
+    vlc_mutex_unlock( &p_sys->lock );

Do you need, sometimes to call rist_Write unlocked?

+static void send_bbnack(stream_t *p_access, int fd_nack, block_t *pkt_nacks, uint16_t nack_count)
+{
+    stream_sys_t *p_sys = p_access->p_sys;
+    struct rist_flow *f = p_sys->f;
+    int len = 0;
+    int bbnack_bufsize = RTCP_FB_HEADER_SIZE +
+        RTCP_FB_FCI_GENERIC_NACK_SIZE * nack_count;
+    block_t *pkt = block_Alloc( bbnack_bufsize );
+    if ( unlikely( pkt == NULL ) )
+    {
+        return;
+    }
+    block_cleanup_push(pkt);
+    /* Populate NACKS */
+    uint8_t *nack = pkt->p_buffer;
+    rtp_set_hdr(nack);
+    rtcp_fb_set_fmt(nack, NACK_FMT_BITMASK);
+    rtcp_set_pt(nack, RTCP_PT_RTPFB);
+    rtcp_set_length(nack, 2 + nack_count);
+    //uint8_t name[4] = "RIST";
+    //rtcp_fb_set_ssrc_media_src(nack, name);
+    len += RTCP_FB_HEADER_SIZE;
+    // TODO : group together
+    uint16_t nacks[MAX_NACKS];
+    memcpy(nacks, pkt_nacks->p_buffer, pkt_nacks->i_buffer);
+    for (int i = 0; i < nack_count; i++) {
+        uint8_t *nack_record = pkt->p_buffer + len + RTCP_FB_FCI_GENERIC_NACK_SIZE*i;
+        rtcp_fb_nack_set_packet_id(nack_record, nacks[i]);
+        rtcp_fb_nack_set_bitmask_lost(nack_record, 0);
+    }
+    len += RTCP_FB_FCI_GENERIC_NACK_SIZE * nack_count;
+    vlc_mutex_lock( &p_sys->lock );
+    rist_WriteTo(fd_nack, pkt->p_buffer, len, (struct sockaddr *)&f->peer_sockaddr, f->peer_socklen);
+    vlc_mutex_unlock( &p_sys->lock );
+    vlc_cleanup_pop();
+    block_Release(pkt);
+    pkt = NULL;
+}

As above for vertical space.
+
+static void send_rbnack(stream_t *p_access, int fd_nack, block_t *pkt_nacks, uint16_t nack_count)
+{
+    stream_sys_t *p_sys = p_access->p_sys;
+    struct rist_flow *f = p_sys->f;
+    int len = 0;
+    int rbnack_bufsize = RTCP_FB_HEADER_SIZE +
+        RTCP_FB_FCI_GENERIC_NACK_SIZE * nack_count;
+    block_t *pkt = block_Alloc( rbnack_bufsize );
+    if ( unlikely( pkt == NULL ) )
+    {
+        return;
+    }
+    block_cleanup_push(pkt);
+    /* Populate NACKS */
+    uint8_t *nack = pkt->p_buffer;
+    rtp_set_hdr(nack);
+    rtcp_fb_set_fmt(nack, NACK_FMT_RANGE);
+    rtcp_set_pt(nack, RTCP_PT_RTPFR);
+    rtcp_set_length(nack, 2 + nack_count);
+    uint8_t name[4] = "RIST";
+    rtcp_fb_set_ssrc_media_src(nack, name);
+    len += RTCP_FB_HEADER_SIZE;
+    // TODO : group together
+    uint16_t nacks[MAX_NACKS];
+    memcpy(nacks, pkt_nacks->p_buffer, pkt_nacks->i_buffer);
+    for (int i = 0; i < nack_count; i++) {
+        uint8_t *nack_record = pkt->p_buffer + len + RTCP_FB_FCI_GENERIC_NACK_SIZE*i;
+        rtcp_fb_nack_set_range_start(nack_record, nacks[i]);
+        rtcp_fb_nack_set_range_extra(nack_record, 0);
+    }
+    len += RTCP_FB_FCI_GENERIC_NACK_SIZE * nack_count;
+    vlc_mutex_lock( &p_sys->lock );
+    rist_WriteTo(fd_nack, pkt->p_buffer, len, (struct sockaddr *)&f->peer_sockaddr, f->peer_socklen);
+    vlc_mutex_unlock( &p_sys->lock );
+    vlc_cleanup_pop();
+    block_Release(pkt);
+    pkt = NULL;
+}
+
+static void send_nacks(stream_t *p_access, struct rist_flow *f)
+{
+    stream_sys_t *p_sys = p_access->p_sys;
+    struct rtp_pkt *pkt;
+    uint16_t idx;
+    uint64_t last_ts = 0;
+    uint16_t null_count = 0;
+    int nacks_len = 0;
+    uint16_t nacks[MAX_NACKS];
+
+    idx = f->ri;
+    while(idx++ != f->wi) {
+        pkt = &(f->buffer[idx]);
+        if (pkt->buffer == NULL) {
+            if (nacks_len + 1 >= MAX_NACKS)
+                break;
+            else {
+                null_count++;
+                // TODO: after adding average spacing calculation, change this formula
+                // to extrapolated_ts = last_ts + null_count * avg_delta_ts;
+                uint64_t extrapolated_ts = last_ts;
+                // Find out the age and add it only if necessary
+                int retry_count = f->nacks_retries[idx];
+                uint64_t age = f->hi_timestamp - extrapolated_ts;
+                uint64_t expiration;
+                if (retry_count == 0)
+                    expiration = f->reorder_buffer;
+                else
+                    expiration = f->nacks_retries[idx] * f->retry_interval;
+                if (age > expiration && retry_count <= f->max_retries) {
+                    f->nacks_retries[idx]++;
+                    nacks[nacks_len++] = idx;
+                    msg_Dbg(p_access, "Sending NACK for seq %d, age %lu ms, retry %u, expiration %lu ms", idx, ts_get_from_rtp(age)/1000, f->nacks_retries[idx], ts_get_from_rtp(expiration)/1000);
+                }
+            }
+        }
+        else
+        {
+            last_ts = pkt->rtp_ts;
+            null_count = 0;
+        }
+    }

Indentation is weird here. Please use 4 spaces.


The rest is much of the same.

Also, split input and output in 2 patches, pelase.

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734


More information about the vlc-devel mailing list