[vlc-devel] [PATCH] Implementation of Dozer streaming protocol: rework 2
Filip Roséen
filip at atch.se
Sun Nov 6 04:18:26 CET 2016
Hi,
The points addressed by *Rémi* related to the patches that affect
`vlc_UrlParse` are still not resolved, and given that these patches
assumes that the necessary support will be added it is rather
cumbersome to review something which one know is going to change (in
one way or another).
I took a few minutes to run through your implementation (in my head, I
did not apply nor compile it). It somewhat saddens me that a lot of
the points brought up in this review was already brought up in the
previous one.
- https://mailman.videolan.org/pipermail/vlc-devel/2016-October/109662.html
I intentionally skipped over some of the sections containing code
that, in one way or another, disagrees with what was addressed in the
previous thread related to your patches.
On 2016-10-30 14:58, Daniele Lacamera wrote:
> In this third set of patches, dozer has been reworked, taken into
> account all the previous remarks. In particular, Jean-Paul has been busy
> fixing most of the issues I failed to detect in the first rework.
>
> As suggested, we are now using vlc_UrlParse to parse the location in the
> access module, but this of course requires the patchset "extend
> vlc_UrlParse() to support udp://addr:port@bindaddr:bindport syntax"
> submitted by Jean-Paul earlier this week. This patchset contains a
> version of the aforementioned, rebased on this morning's master, and
> containing a few of the additional minor fixes proposed by Filip.
>
> Comments are welcome.
> From 62728898706989c42d59889f18dcccba7dda70f6 Mon Sep 17 00:00:00 2001
> From: Daniele Lacamera <root at danielinux.net>
> Date: Sun, 30 Oct 2016 13:31:42 +0100
> Subject: [PATCH 6/6] Added dozer access_output module
>
> ---
> modules/access_output/Makefile.am | 5 +-
> modules/access_output/dozer.c | 721 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 725 insertions(+), 1 deletion(-)
> create mode 100644 modules/access_output/dozer.c
>
> diff --git a/modules/access_output/Makefile.am b/modules/access_output/Makefile.am
> index 7c93116..03deb12 100644
> --- a/modules/access_output/Makefile.am
> +++ b/modules/access_output/Makefile.am
> @@ -6,12 +6,15 @@ libaccess_output_file_plugin_la_LIBADD = $(LIBPTHREAD)
> libaccess_output_http_plugin_la_SOURCES = access_output/http.c
> libaccess_output_udp_plugin_la_SOURCES = access_output/udp.c
> libaccess_output_udp_plugin_la_LIBADD = $(SOCKET_LIBS) $(LIBPTHREAD)
> +libaccess_output_dozer_plugin_la_SOURCES = access_output/dozer.c
> +libaccess_output_dozer_plugin_la_LIBADD = $(SOCKET_LIBS) $(LIBPTHREAD)
>
> access_out_LTLIBRARIES = \
> libaccess_output_dummy_plugin.la \
> libaccess_output_file_plugin.la \
> libaccess_output_http_plugin.la \
> - libaccess_output_udp_plugin.la
> + libaccess_output_udp_plugin.la \
> + libaccess_output_dozer_plugin.la
>
> libaccess_output_livehttp_plugin_la_SOURCES = access_output/livehttp.c
> libaccess_output_livehttp_plugin_la_CFLAGS = $(AM_CFLAGS) $(GCRYPT_CFLAGS)
> diff --git a/modules/access_output/dozer.c b/modules/access_output/dozer.c
> new file mode 100644
> index 0000000..da00f3b
> --- /dev/null
> +++ b/modules/access_output/dozer.c
> @@ -0,0 +1,721 @@
> +/*****************************************************************************
> + * dozer.c
> + *****************************************************************************
> + * Copyright (C) 2016 Daniele Lacamera, Sergio Ammirata, Jean-Paul Saman
> + * $Id$
> + *
> + * Authors: Daniele Lacamera <root at danielinux.net>
> + * Sergio Ammirata <sergio at ammirata.net>
> + * Jean-Paul Saman <jpsaman at videolan.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> + *****************************************************************************/
> +
> +/*****************************************************************************
> + * Preamble
> + *****************************************************************************/
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <errno.h>
> +
> +#include <vlc_sout.h>
> +#include <vlc_block.h>
> +#include <vlc_codec.h>
> +
> +#ifdef _WIN32
> +# include <winsock2.h>
> +# include <ws2tcpip.h>
> +#else
> +# include <sys/socket.h>
> +#endif
> +
> +#include <vlc_network.h>
> +
> +#define MAX_EMPTY_BLOCKS 200
> +
> +#define SIGNATURE 0xD023
> +
> +
> +/* Fec data */
> +
> +#define DEFAULT_DOZER_FEC_ROWS 5
> +#define DEFAULT_DOZER_FEC_COLS 10
> +
> +#define FEC_TYPE_DATA 0x0000
> +#define FEC_TYPE_FEC_ROW 0x1000
> +#define FEC_TYPE_FEC_COL 0x2000
> +
> +static uint64_t i_seq = 0;
> +
> +#pragma pack(push,1)
> +struct dozer_hdr {
> + uint16_t signature;
> + uint16_t type;
> + uint16_t seq;
> + uint16_t win;
> + uint16_t count;
> + uint16_t size;
> +};
> +#pragma pack(pop)
> +
> +
> +/*****************************************************************************
> + * Module descriptor
> + *****************************************************************************/
> +static int Open ( vlc_object_t *p_this );
> +static void Close( vlc_object_t *p_this );
> +
> +#define SOUT_CFG_PREFIX "sout-dozer-"
> +
> +#define COLS_TEXT N_("FEC 2022 Columns")
> +#define COLS_LONGTEXT N_( \
> + "Number of Columns to use for the FEC 2022 matrix. Default is 10 cols and" \
> + " 5 rows for 30% overhead and about 3% packet loss recovery" )
> +
> +#define ROWS_TEXT N_("FEC 2022 Rows")
> +#define ROWS_LONGTEXT N_( \
> + "Number of Rows to use for the FEC 2022 matrix. Default is 5 rows and" \
> + " 10 cols for 30% overhead and about 3% packet loss recovery" )
> +
> +#define CACHING_TEXT N_("Caching value (ms)")
> +#define CACHING_LONGTEXT N_( \
> + "Default caching value for outbound UDP Dozer streams. This " \
> + "value should be set in milliseconds." )
> +
> +#define GROUP_TEXT N_("Group packets")
> +#define GROUP_LONGTEXT N_("Packets can be sent one by one at the right time " \
> + "or by groups. You can choose the number " \
> + "of packets that will be sent at a time. It " \
> + "helps reducing the scheduling load on " \
> + "heavily-loaded systems." )
> +
> +#define DOZER_LONGTEXT N_("Dozer output module: This module provides an implementation " \
> + "of a subset of the dozer network protocol for tranmission of udp streams " \
> + "over lossy networks. The protocol is UDP based and the recovery algorithm " \
> + "is based on SMPTE2022. " \
> + "In this implementation, the buffer/delay size is fixed and equal to " \
> + "1316 * 8 * cols * rows / bps; For example, for a 1Mbps stream with the " \
> + "default 10x5 matrix with 30% overhead, we would have a 500ms buffer and " \
> + "and an adequate recovery of up to 3% packet loss (packet loss recovery "\
> + "estimate comes from empirical measurements)." )
The above will *suck* for translators, and all the information in
`DOZER_LONGTEXT` is in my opinion not necessary.
Simply stating that the Dozer module implements a subset of the *dozer
network protocol* with *some* added information is enough (but the
current message is too verbose imo).
> +
> +vlc_module_begin ()
> + set_description( DOZER_LONGTEXT )
> + set_shortname( "DOZER" )
> + set_category( CAT_SOUT )
> + set_subcategory( SUBCAT_SOUT_ACO )
> + add_integer( SOUT_CFG_PREFIX "caching", DEFAULT_PTS_DELAY / 1000,
> + CACHING_TEXT, CACHING_LONGTEXT, true )
> +
> + add_integer( SOUT_CFG_PREFIX "group", 1, GROUP_TEXT, GROUP_LONGTEXT, true )
> + add_integer( SOUT_CFG_PREFIX "rows", DEFAULT_DOZER_FEC_ROWS, ROWS_TEXT,
> + ROWS_LONGTEXT, true)
> +
> + add_integer( SOUT_CFG_PREFIX "cols", DEFAULT_DOZER_FEC_COLS, COLS_TEXT,
> + COLS_LONGTEXT, true)
> +
> + set_capability( "sout access", 0 )
> + add_shortcut( "dozer", "dozer2022" )
> + set_callbacks( Open, Close )
> +vlc_module_end ()
> +
> +/*****************************************************************************
> + * Exported prototypes
> + *****************************************************************************/
> +
> +static const char *const ppsz_sout_options[] = {
> + "caching",
> + "group",
> + "rows",
> + "cols",
> + NULL
> +};
> +
> +/* Options handled by the libvlc network core */
> +static const char *const ppsz_core_options[] = {
> + "dscp",
> + "ttl",
> + "miface",
> + NULL
> +};
> +
> +static ssize_t Write ( sout_access_out_t *, block_t * );
> +static int Seek ( sout_access_out_t *, off_t );
> +static int Control( sout_access_out_t *, int, va_list );
> +
> +static void* ThreadWrite( void * );
> +static block_t *NewUDPPacket( sout_access_out_t *, mtime_t );
> +
> +struct sout_access_out_sys_t
> +{
> + mtime_t i_caching;
> + int i_handle;
> + bool b_mtu_warning;
> + size_t i_mtu;
> +
> + block_fifo_t *p_fifo;
> + block_fifo_t *p_empty_blocks;
> + block_t *p_buffer;
> +
> + block_t **fec_rows;
> + block_t **fec_cols;
> +
> + int i_rows;
> + int i_cols;
> + int i_fecsize;
Unnecessary whitespace, the implementation would actually be more
readable if it looked like the below.
block_t **fec_rows;
block_t **fec_cols;
int i_rows;
int i_cols;
int i_fecsize;
> +
> + vlc_thread_t thread;
> +};
> +
> +/*****************************************************************************
> + * FEC functions
> + *****************************************************************************/
> +
> +/* Exclude the dozer header from the FEC algorithm. Include only the last field
> + * (size) as 'check' field for content mismatch.
> + */
> +#define HDR_EXCLUDE ((sizeof(struct dozer_hdr) - (sizeof(uint16_t))))
> +
> +/* FEC XOR function *
> + *
> + * Compute XOR between two packets f1 and f2. New packet is stored in f1.
> + *
> + */
> +static void fec_xor(sout_access_out_t *access, block_t *f1, const block_t *f2)
> +{
> + /* Check if f1 is still empty and needs initiailization */
> + if (f1->i_buffer == 0) {
> + memset(f1->p_buffer, 0, sizeof(struct dozer_hdr));
> + f1->i_buffer = f2->i_buffer;
> + memcpy(f1->p_buffer + HDR_EXCLUDE, f2->p_buffer + HDR_EXCLUDE, f1->i_buffer - HDR_EXCLUDE);
> + }
> + else if (f1->i_buffer != f2->i_buffer) {
> + msg_Warn(access, "XOR operation failed: buffer size mismatch.\n");
> + }
> + else if (f1->i_buffer < sizeof(struct dozer_hdr)) {
> + msg_Warn(access, "XOR operation failed: buffer size too short.\n");
> + } else {
> + for(size_t i = HDR_EXCLUDE; i < f1->i_buffer; i++)
> + f1->p_buffer[i] ^= f2->p_buffer[i];
> + }
> +}
> +
> +/*****************************************************************************
> + * Open: open the file
> + *****************************************************************************/
> +#define DEFAULT_PORT 1236
> +static int Open( vlc_object_t *p_this )
> +{
> + sout_access_out_t *p_access = (sout_access_out_t*)p_this;
> + sout_access_out_sys_t *p_sys;
> +
> + char *psz_dst_addr = NULL;
> + int i_dst_port;
> +
> + config_ChainParse( p_access, SOUT_CFG_PREFIX,
> + ppsz_sout_options, p_access->p_cfg );
> + config_ChainParse( p_access, "",
> + ppsz_core_options, p_access->p_cfg );
> +
> + if (var_Create (p_access, "dst-port", VLC_VAR_INTEGER)
> + || var_Create (p_access, "src-port", VLC_VAR_INTEGER)
> + || var_Create (p_access, "dst-addr", VLC_VAR_STRING)
> + || var_Create (p_access, "src-addr", VLC_VAR_STRING))
> + {
> + return VLC_ENOMEM;
> + }
> +
> + if( !( p_sys = malloc( sizeof( *p_sys ) ) ) )
> + return VLC_ENOMEM;
> + p_access->p_sys = p_sys;
> +
> + i_dst_port = DEFAULT_PORT;
> + char *psz_parser = psz_dst_addr = strdup( p_access->psz_path );
> + if( !psz_dst_addr )
> + {
> + free( p_sys );
> + return VLC_ENOMEM;
> + }
> +
> + if (psz_parser[0] == '[')
> + psz_parser = strchr (psz_parser, ']');
> +
> + psz_parser = strchr (psz_parser ? psz_parser : psz_dst_addr, ':');
> + if (psz_parser != NULL)
> + {
> + *psz_parser++ = '\0';
> + i_dst_port = atoi (psz_parser);
> + }
> +
> + p_sys->i_handle = net_ConnectDgram( p_this, psz_dst_addr, i_dst_port, -1,
> + IPPROTO_UDP );
> + free (psz_dst_addr);
> +
> + if( p_sys->i_handle == -1 )
> + {
> + msg_Err( p_access, "failed to create raw UDP socket" );
> + free (p_sys);
> + return VLC_EGENERIC;
> + }
> + else
> + {
> + char addr[NI_MAXNUMERICHOST];
> + int port;
> +
> + if (net_GetSockAddress (p_sys->i_handle, addr, &port) == 0)
> + {
> + msg_Dbg (p_access, "source: %s port %d", addr, port);
> + var_SetString (p_access, "src-addr", addr);
> + var_SetInteger (p_access, "src-port", port);
> + }
> +
> + if (net_GetPeerAddress (p_sys->i_handle, addr, &port) == 0)
> + {
> + msg_Dbg (p_access, "destination: %s port %d", addr, port);
> + var_SetString (p_access, "dst-addr", addr);
> + var_SetInteger (p_access, "dst-port", port);
> + }
> + }
> + shutdown( p_sys->i_handle, SHUT_RD );
> +
> + p_sys->i_rows = var_GetInteger( p_access, SOUT_CFG_PREFIX "rows" );
> + p_sys->i_cols = var_GetInteger( p_access, SOUT_CFG_PREFIX "cols" );
> + p_sys->i_fecsize = p_sys->i_rows * p_sys->i_cols;
> + p_sys->i_caching = UINT64_C(1000)
> + * var_GetInteger( p_access, SOUT_CFG_PREFIX "caching");
> + p_sys->i_mtu = var_CreateGetInteger( p_this, "mtu" );
> + p_sys->b_mtu_warning = false;
> + p_sys->p_buffer = NULL;
> + p_sys->p_empty_blocks = NULL;
> + p_sys->p_fifo = block_FifoNew();
> + if (p_sys->p_fifo == NULL) {
> + msg_Err( p_access, "cannot allocate fifo" );
> + goto error;
> + }
> + p_sys->p_empty_blocks = block_FifoNew();
> + if (p_sys->p_empty_blocks == NULL) {
> + msg_Err( p_access, "cannot allocate fifo" );
> + goto error;
> + }
> + if (p_sys->i_rows == 0 || p_sys->i_cols == 0) {
> + msg_Err( p_access, "Invalid size specified for FEC matrix" );
> + goto error;
> + }
> + p_sys->fec_rows = calloc(p_sys->i_rows, sizeof(block_t *));
> + if (!p_sys->fec_rows) {
> + msg_Err( p_access, "cannot allocate FEC matrix" );
> + goto error;
> + }
> + p_sys->fec_cols = calloc(p_sys->i_cols, sizeof(block_t *));
> + if (!p_sys->fec_cols) {
> + msg_Err( p_access, "cannot allocate FEC matrix" );
> + goto error;
> + }
> + if( vlc_clone( &p_sys->thread, ThreadWrite, p_access,
> + VLC_THREAD_PRIORITY_HIGHEST ) )
> + {
> + msg_Err( p_access, "cannot spawn sout access thread" );
> + goto error;
> + }
> +
> + p_access->pf_write = Write;
> + p_access->pf_seek = Seek;
> + p_access->pf_control = Control;
> +
> + return VLC_SUCCESS;
> +
> +error:
> + block_FifoRelease( p_sys->p_fifo );
> + if (p_sys->p_empty_blocks)
> + block_FifoRelease( p_sys->p_empty_blocks );
> + if (p_sys->fec_rows) free(p_sys->fec_rows);
> + if (p_sys->fec_cols) free(p_sys->fec_cols);
The above two `if` are redundant; `free` can be called with `NULL`.
> + net_Close(p_sys->i_handle);
> + free (p_sys);
> + return VLC_ENOMEM;
> +}
> +
> +/*****************************************************************************
> + * Close: close the target
> + *****************************************************************************/
> +static void Close( vlc_object_t * p_this )
> +{
> + sout_access_out_t *p_access = (sout_access_out_t*)p_this;
> + sout_access_out_sys_t *p_sys = p_access->p_sys;
> +
> + vlc_cancel( p_sys->thread );
> + vlc_join( p_sys->thread, NULL );
> + block_FifoRelease( p_sys->p_fifo );
> + block_FifoRelease( p_sys->p_empty_blocks );
> +
> + if (p_sys->fec_rows) {
> + for (int i = 0; i < p_sys->i_rows; i++)
> + if (p_sys->fec_rows[i]) block_Release(p_sys->fec_rows[i]);
> + free(p_sys->fec_rows);
> + p_sys->fec_rows = NULL;
> + }
> + if (p_sys->fec_cols) {
> + for (int i = 0; i < p_sys->i_cols; i++)
> + if (p_sys->fec_cols[i]) block_Release(p_sys->fec_cols[i]);
> + free(p_sys->fec_cols);
> + p_sys->fec_cols = NULL;
> + }
*code-duplication*: the above could be refactored into a single
*helper-function* given that the type of `p_sys->fec_rows` and
p_sys->fec_cols` are the same.
> + if( p_sys->p_buffer ) block_Release( p_sys->p_buffer );
Please do not cram things on a single line for no apparent reason,
code-readability is harmed when trying to keep the *LOC* down for the
single reason of trying to keep the *LOC* down.
> +
> + net_Close( p_sys->i_handle );
> + free( p_sys );
> +}
> +
> +static int Control( sout_access_out_t *p_access, int i_query, va_list args )
> +{
> + (void)p_access;
> +
> + switch( i_query )
> + {
> + case ACCESS_OUT_CONTROLS_PACE:
> + *va_arg( args, bool * ) = false;
> + break;
> +
> + default:
> + return VLC_EGENERIC;
> + }
> + return VLC_SUCCESS;
> +}
> +
> +/*****************************************************************************
> + * Write: standard write on a file descriptor.
> + *****************************************************************************/
> +
> +static void reset_fec(sout_access_out_t *p_access)
> +{
> + sout_access_out_sys_t *p_sys = p_access->p_sys;
> + mtime_t now = mdate();
> + for (int i = 0; i < p_sys->i_rows; i++) {
> + p_access->p_sys->fec_rows[i] = NewUDPPacket(p_access, now);
> + }
> + for (int j = 0; j < p_sys->i_cols; j++) {
> + p_access->p_sys->fec_cols[j] = NewUDPPacket(p_access, now);
> + }
> +}
> +
> +static block_t *process_fec_row(sout_access_out_t *p_access, block_t *p_buffer, uint64_t seq)
> +{
> + sout_access_out_sys_t *p_sys = p_access->p_sys;
> + uint16_t win = seq / p_sys->i_fecsize;
> + uint16_t iseq = seq % p_sys->i_fecsize;
> + uint16_t row = iseq / p_sys->i_cols;
> + uint16_t col = iseq % p_sys->i_cols;
> +
> + if (iseq == 0) {
> + reset_fec(p_access);
> + }
> +
> + if (unlikely(!p_access->p_sys->fec_rows[row])) {
> + msg_Warn(p_access, "Updating FEC: Invalid ROW FEC block found. \n");
> + return NULL;
> + }
> +
> + /* Update row fec */
> + fec_xor(p_access, p_access->p_sys->fec_rows[row], p_buffer);
> + /* Last in row? Enqueue. */
> + if (col == (p_sys->i_cols - 1)) {
> + block_t *fec = p_access->p_sys->fec_rows[row];
> + struct dozer_hdr *hdr = (struct dozer_hdr *)fec->p_buffer;
> + hdr->signature = htons(SIGNATURE);
> + hdr->type = htons(FEC_TYPE_FEC_ROW);
> + hdr->win = htons(win);
> + hdr->seq = htons(row);
> + hdr->count = htons(p_sys->i_rows);
> + fec->i_dts = p_buffer->i_dts;
> + return fec;
> + }
> + return NULL;
> +}
`process_fec_col` (below) and `process_fec_row` (above) look like a
direct *copy-paste* where only a minor subset of the implementation
has changed.
It would be a lot cleaner to have a single helper-function that can be
invoked for both (avoiding *code-duplication* which leads to harder
maintenance).
> +static block_t *process_fec_col(sout_access_out_t *p_access, block_t *p_buffer, uint64_t seq)
> +{
> + sout_access_out_sys_t *p_sys = p_access->p_sys;
> + uint16_t win = seq / p_sys->i_fecsize;
> + uint16_t iseq = seq % p_sys->i_fecsize;
> + uint16_t row = iseq / p_sys->i_cols;
> + uint16_t col = iseq % p_sys->i_cols;
> +
> + if (unlikely(!p_access->p_sys->fec_cols[col])) {
> + msg_Warn(p_access, "Updating FEC: Invalid COL FEC block found. \n");
> + return NULL;
> + }
> +
> + /* Update col fec */
> + fec_xor(p_access, p_access->p_sys->fec_cols[col], p_buffer);
> + /* Last row? Enqueue. */
> + if (row == (p_sys->i_rows - 1)) {
> + block_t *fec = p_access->p_sys->fec_cols[col];
> + struct dozer_hdr *hdr = (struct dozer_hdr *)fec->p_buffer;
> + hdr->signature = htons(SIGNATURE);
> + hdr->type = htons(FEC_TYPE_FEC_COL);
> + hdr->win = htons(win);
> + hdr->seq = htons(col);
> + hdr->count = htons(p_sys->i_cols);
> + fec->i_dts = p_buffer->i_dts;
> + return fec;
> + }
> + return NULL;
> +}
> +
> +static ssize_t Write( sout_access_out_t *p_access, block_t *p_buffer )
> +{
> + sout_access_out_sys_t *p_sys = p_access->p_sys;
> + int i_len = 0;
> +
> + while( p_buffer )
> + {
> + block_t *p_next;
> + int i_packets = 0;
> + mtime_t now = mdate();
> + struct dozer_hdr *hdr;
> + block_t *fec_c, *fec_r;
> +
> + if (unlikely( !p_sys->b_mtu_warning && p_buffer->i_buffer > p_sys->i_mtu ))
> + {
> + msg_Warn( p_access, "packet size > MTU, you should probably "
> + "increase the MTU" );
This diagnostic is only helpful if you also include how big the packet
is (that is larger than `p_sys->i_mtu`).
> + p_sys->b_mtu_warning = true;
> + }
> +
> + /* Check if there is enough space in the buffer */
> + if ( p_sys->p_buffer &&
> + p_sys->p_buffer->i_buffer + p_buffer->i_buffer > p_sys->i_mtu )
> + {
> + if(unlikely( p_sys->p_buffer->i_dts + p_sys->i_caching < now ))
> + {
> + msg_Dbg( p_access, "late packet for UDP input (%"PRId64 ")",
> + now - p_sys->p_buffer->i_dts
> + - p_sys->i_caching );
> + }
> + /* Populate header */
> + hdr = (struct dozer_hdr *)(p_sys->p_buffer->p_buffer);
This may lead to alignment issues as I do not see anywhere where you
make sure that `p_sys->p_buffer->p_buffer` is aligned to a boundary at
least that of `uint16_t`.
This applies to every location where you make such cast.
> + hdr->size = htons(p_sys->p_buffer->i_buffer - sizeof(struct dozer_hdr));
> + fec_r = process_fec_row(p_access, p_sys->p_buffer, i_seq - 1);
> + fec_c = process_fec_col(p_access, p_sys->p_buffer, i_seq - 1);
> + block_FifoPut( p_sys->p_fifo, p_sys->p_buffer );
> + p_sys->p_buffer = NULL;
> + if (fec_r)
> + block_FifoPut(p_sys->p_fifo, fec_r);
> + if (fec_c)
> + block_FifoPut(p_sys->p_fifo, fec_c);
> + }
> +
> + i_len += p_buffer->i_buffer;
> + while( p_buffer->i_buffer )
> + {
> + size_t i_payload_size = p_sys->i_mtu;
> + size_t i_write = __MIN( p_buffer->i_buffer, i_payload_size );
> +
> + i_packets++;
> +
> + if( !p_sys->p_buffer )
> + {
> + p_sys->p_buffer = NewUDPPacket( p_access, p_buffer->i_dts );
> + if( !p_sys->p_buffer )
> + break;
> +
> + /* Populate header */
> + hdr = (struct dozer_hdr *)(p_sys->p_buffer->p_buffer);
> + hdr->signature = htons(SIGNATURE);
> + hdr->type = htons(FEC_TYPE_DATA);
> + hdr->seq = htons(i_seq % p_sys->i_fecsize);
> + hdr->win = htons((i_seq / p_sys->i_fecsize) & 0xFFFF);
> + i_seq++;
> + p_sys->p_buffer->i_buffer += sizeof(struct dozer_hdr);
> + } else
> + hdr = (struct dozer_hdr *)(p_sys->p_buffer->p_buffer);
> +
> + memcpy( p_sys->p_buffer->p_buffer + p_sys->p_buffer->i_buffer,
> + p_buffer->p_buffer, i_write );
> +
> + p_sys->p_buffer->i_buffer += i_write;
> + p_buffer->p_buffer += i_write;
> + p_buffer->i_buffer -= i_write;
> + if (unlikely( p_buffer->i_flags & BLOCK_FLAG_CLOCK ))
> + {
> + if ( p_sys->p_buffer->i_flags & BLOCK_FLAG_CLOCK )
> + msg_Warn( p_access, "putting two PCRs at once" );
> + p_sys->p_buffer->i_flags |= BLOCK_FLAG_CLOCK;
> + }
> +
> + if ( p_sys->p_buffer->i_buffer == p_sys->i_mtu || i_packets > 1 )
> + {
> + /* Flush */
> + if( p_sys->p_buffer->i_dts + p_sys->i_caching < now )
> + {
> + msg_Dbg( p_access, "late packet for udp input (%"PRId64 ")",
> + mdate() - p_sys->p_buffer->i_dts
> + - p_sys->i_caching );
> + }
> + /* populate packet size */
> + hdr->size = htons(p_sys->p_buffer->i_buffer - sizeof(struct dozer_hdr));
> + fec_r = process_fec_row(p_access, p_sys->p_buffer, i_seq - 1);
> + fec_c = process_fec_col(p_access, p_sys->p_buffer, i_seq - 1);
> + block_FifoPut( p_sys->p_fifo, p_sys->p_buffer );
> + p_sys->p_buffer = NULL;
> + if (fec_r)
> + block_FifoPut(p_sys->p_fifo, fec_r);
> + if (fec_c)
> + block_FifoPut(p_sys->p_fifo, fec_c);
> + }
> + }
> +
> + p_next = p_buffer->p_next;
> + block_Release( p_buffer );
> + p_buffer = p_next;
> + }
> + return i_len;
> +}
> +
> +/*****************************************************************************
> + * Seek: seek to a specific location in a file
> + *****************************************************************************/
> +static int Seek( sout_access_out_t *p_access, off_t i_pos )
> +{
> + (void) i_pos;
> + msg_Err( p_access, "Dozer sout access cannot seek" );
> + return -1;
> +}
> +
> +/*****************************************************************************
> + * NewUDPPacket: allocate a new UDP packet of size p_sys->i_mtu
> + *
> + * If allocation fails, returns NULL. Caller must check return value before
> + * dereferencing.
> + *****************************************************************************/
> +static block_t *NewUDPPacket( sout_access_out_t *p_access, mtime_t i_dts)
> +{
> + sout_access_out_sys_t *p_sys = p_access->p_sys;
> + block_t *p_buffer;
> +
> + while ( block_FifoCount( p_sys->p_empty_blocks ) > MAX_EMPTY_BLOCKS )
> + {
> + p_buffer = block_FifoGet( p_sys->p_empty_blocks );
> + block_Release( p_buffer );
> + }
> +
> + if( block_FifoCount( p_sys->p_empty_blocks ) == 0 )
> + {
> + p_buffer = block_Alloc( p_sys->i_mtu + sizeof(struct dozer_hdr));
> + }
> + else
> + {
> + p_buffer = block_FifoGet(p_sys->p_empty_blocks );
> + p_buffer->i_flags = 0;
> + p_buffer = block_Realloc( p_buffer, 0, p_sys->i_mtu + sizeof(struct dozer_hdr));
> + }
> +
> + if (p_buffer) {
> + p_buffer->i_dts = i_dts;
> + p_buffer->i_buffer = 0;
> + }
> +
> + return p_buffer;
> +}
> +
> +/*****************************************************************************
> + * ThreadWrite: Write a packet on the network at the good time.
> + *****************************************************************************/
> +static void* ThreadWrite( void *data )
> +{
> + sout_access_out_t *p_access = data;
> + sout_access_out_sys_t *p_sys = p_access->p_sys;
> + mtime_t i_date_last = -1;
> + const unsigned i_group = var_GetInteger( p_access,
> + SOUT_CFG_PREFIX "group" );
> + mtime_t i_to_send = i_group;
> + unsigned i_dropped_packets = 0;
> + struct dozer_hdr *hdr;
> + uint16_t dozer_type;
> + msg_Dbg( p_access, "Dozer: Sending thread started.");
> +
> + for (;;)
> + {
> + block_t *p_pk = block_FifoGet( p_sys->p_fifo );
> + mtime_t i_date, i_sent;
> +
> + if (unlikely(!p_pk))
> + return NULL;
> +
> + hdr = (struct dozer_hdr *)(p_pk->p_buffer);
> + dozer_type = ntohs(hdr->type);
> +
> + if (dozer_type != FEC_TYPE_DATA) {
> + block_cleanup_push( p_pk );
> + if ( send( p_sys->i_handle, p_pk->p_buffer, p_pk->i_buffer, 0 ) == -1 )
> + msg_Warn( p_access, "send error: %s", vlc_strerror_c(errno) );
> + vlc_cleanup_pop();
> + block_FifoPut( p_sys->p_empty_blocks, p_pk );
> + continue;
> + }
> +
> + i_date = p_sys->i_caching + p_pk->i_dts;
> + if( i_date_last > 0 ) {
> + if( i_date - i_date_last > 2000000 ) {
> + if( !i_dropped_packets )
> + msg_Dbg( p_access, "mmh, hole (%"PRId64" > 2s) -> drop",
> + i_date - i_date_last );
> +
> + block_FifoPut( p_sys->p_empty_blocks, p_pk );
> + i_date_last = i_date;
> + i_dropped_packets++;
> + continue;
> + } else if( i_date - i_date_last < -1000 ) {
> + if( !i_dropped_packets )
> + msg_Dbg( p_access, "mmh, packets in the past (%"PRId64")",
> + i_date_last - i_date );
> + }
> + }
> + block_cleanup_push( p_pk );
> + i_to_send--;
> + if( !i_to_send || (p_pk->i_flags & BLOCK_FLAG_CLOCK) )
> + {
> + mwait( i_date );
> + i_to_send = i_group;
> + }
> + if ( send( p_sys->i_handle, p_pk->p_buffer, p_pk->i_buffer, 0 ) == -1 )
> + msg_Warn( p_access, "send error: %s", vlc_strerror_c(errno) );
> + vlc_cleanup_pop();
> +
> + if( i_dropped_packets )
> + {
> + msg_Dbg( p_access, "dropped %i packets", i_dropped_packets );
> + i_dropped_packets = 0;
> + }
> +
> + i_sent = mdate();
> + if ( i_sent > i_date + 20000 )
> + {
> + msg_Dbg( p_access, "packet has been sent too late (%"PRId64 ")",
> + i_sent - i_date );
> + }
> + block_FifoPut( p_sys->p_empty_blocks, p_pk );
> + i_date_last = i_date;
> + }
> + return NULL;
The above looks like a *copy-paste* of `modules/access_output/udp.c`,
and if one is to nitpick credit should be given to those who deserve
it.
> +}
> --
> 2.8.1
>
> From d720b388931c6f8752cb1312b1d39e77d9d5ee83 Mon Sep 17 00:00:00 2001
> From: Daniele Lacamera <root at danielinux.net>
> Date: Sun, 30 Oct 2016 13:31:19 +0100
> Subject: [PATCH 5/6] Added dozer access module
>
> ---
> modules/access/Makefile.am | 4 +
> modules/access/dozer.c | 775 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 779 insertions(+)
> create mode 100644 modules/access/dozer.c
>
> diff --git a/modules/access/Makefile.am b/modules/access/Makefile.am
> index 061f2b1..cbd0b57 100644
> --- a/modules/access/Makefile.am
> +++ b/modules/access/Makefile.am
> @@ -401,6 +401,10 @@ libudp_plugin_la_SOURCES = access/udp.c
> libudp_plugin_la_LIBADD = $(SOCKET_LIBS) $(LIBPTHREAD)
> access_LTLIBRARIES += libudp_plugin.la
>
> +libdozer_plugin_la_SOURCES = access/dozer.c
> +libdozer_plugin_la_LIBADD = $(SOCKET_LIBS) $(LIBPTHREAD)
> +access_LTLIBRARIES += libdozer_plugin.la
> +
> libsftp_plugin_la_SOURCES = access/sftp.c
> libsftp_plugin_la_CFLAGS = $(AM_CFLAGS) $(SFTP_CFLAGS)
> libsftp_plugin_la_LIBADD = $(SFTP_LIBS)
> diff --git a/modules/access/dozer.c b/modules/access/dozer.c
> new file mode 100644
> index 0000000..40398f8
> --- /dev/null
> +++ b/modules/access/dozer.c
> @@ -0,0 +1,775 @@
> +/*****************************************************************************
> + * dozer.c
> + *****************************************************************************
> + * Copyright (C) 2016 Daniele Lacamera, Sergio Ammirata, Jean-Paul Saman
> + * $Id$
> + *
> + * Authors: Daniele Lacamera <root at danielinux.net>
> + * Sergio Ammirata <sergio at ammirata.net>
> + * Jean-Paul Saman <jpsaman at videolan.org>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> + *****************************************************************************/
> +
> +/*****************************************************************************
> + * Preamble
> + *****************************************************************************/
> +
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +#include <vlc_access.h>
> +#include <vlc_network.h>
> +#include <vlc_block.h>
> +#include <vlc_interrupt.h>
> +#include <vlc_atomic.h>
> +#include <vlc_codec.h>
> +#include <vlc_url.h>
> +#include <dvb/dvb.h> /* For TS_PACKET_SIZE */
> +#ifdef HAVE_POLL
> +# include <poll.h>
> +#endif
> +#include <fcntl.h>
> +
> +#define SIGNATURE 0xD023
> +
> +/* Fec data */
> +#define FEC_TYPE_DATA 0x0000
> +#define FEC_TYPE_FEC_ROW 0x1000
> +#define FEC_TYPE_FEC_COL 0x2000
> +
> +enum fec_recover_type {
> + RECOVER_ROW =1,
> + RECOVER_COL
> +};
I fail to see the reason for explicitly giving `RECOVER_ROW` a value,
when the enumerators are simply used for direct comparison. Explicitly
giving a value indicates that the enumerator will be used for
something else (which does not seem to be the case).
> +
> +/* Maximum allowed value for matrix rows and columns. */
> +#define FEC_MATRIX_MAX 50
> +
> +#pragma pack(push,1)
> +struct dozer_hdr {
> + uint16_t signature;
> + uint16_t type;
> + uint16_t seq;
> + uint16_t win;
> + uint16_t count;
> + uint16_t size;
> +};
> +#pragma pack(pop)
As stated previously; the *packing* of a `struct`'s *data-members* is
*implementation-specific*, you should not rely on it (as nothing
guarantees that it will be honored, or even compile).
As with many things showing up in this review, I mentioned it in my
previous one.
> +
> +
> +/*****************************************************************************
> + * Module descriptor
> + *****************************************************************************/
> +static int Open( vlc_object_t * );
> +static void Close( vlc_object_t * );
> +
> +#define BUFFER_TEXT N_("UDP Receive buffer")
> +#define BUFFER_LONGTEXT N_("UDP receive buffer size (bytes)" )
> +#define TIMEOUT_TEXT N_("UDP Source timeout (sec)")
> +
> +#define DOZER_LONGTEXT N_("Dozer input module: This module provides an implementation " \
> + "of a subset of the dozer network protocol for tranmission of udp streams " \
> + "over lossy networks. The protocol is UDP based and the recovery algorithm " \
> + "is based on SMPTE2022. " \
> + "In this implementation, the buffer/delay size is fixed and equal to " \
> + "1316 * 8 * cols * rows / bps; For example, for a 1Mbps stream with the " \
> + "default 10x5 matrix with 30% overhead, we would have a 500ms buffer and " \
> + "and an adequate recovery of up to 3% packet loss (packet loss recovery "\
> + "estimate comes from empirical measurements)." )
Once again; this text will be a pain to translate, and honestly
contains a lot more information than necessary.
> +
> +#define DEFAULT_DOZER_UDP_BUFFER 0x400000
> +
> +vlc_module_begin ()
> + set_shortname( N_("DOZER" ) )
> + set_description( DOZER_LONGTEXT )
> + set_category( CAT_INPUT )
> + set_subcategory( SUBCAT_INPUT_ACCESS )
> +
> + add_integer( "udp-buffer", DEFAULT_DOZER_UDP_BUFFER, BUFFER_TEXT, BUFFER_LONGTEXT, true )
> + add_integer( "udp-timeout", -1, TIMEOUT_TEXT, NULL, true )
> +
> + set_capability( "access", 0 )
> + add_shortcut( "dozer", "dozer2022" )
> +
> + set_callbacks( Open, Close )
> +vlc_module_end ()
> +
> +struct access_sys_t
> +{
> + int fd;
> + int timeout;
> + size_t mtu;
> + size_t fifo_size;
> + block_fifo_t *fifo;
> +
> + block_t **fec_cols;
> + block_t **fec_rows;
> + block_t **matrix;
> +
> + /* Matrix size */
> + uint16_t n_rows;
> + uint16_t n_cols;
> + uint16_t n_tot;
> +
> + /* Counters/flags */
> + uint16_t fec_cur_win;
> + uint16_t fec_last_rx;
> + uint16_t fec_initialized;
> +
> + vlc_sem_t semaphore;
> + vlc_thread_t thread;
> + atomic_bool timeout_reached;
> +};
> +
> +/*****************************************************************************
> + * Local prototypes
> + *****************************************************************************/
> +static block_t *BlockUDP( access_t *p_access, bool *restrict eof );
> +static int Control( access_t *p_access, int i_query, va_list args);
> +static void* ThreadRead( void *data );
> +
> +/*****************************************************************************
> + * FEC functions
> + *****************************************************************************/
> +/* Exclude the dozer header from the FEC algorithm. Include only the last field
> + * (size) as 'check' field for content mismatch.
> + */
> +#define HDR_EXCLUDE ((sizeof(struct dozer_hdr) - (sizeof(uint16_t))))
> +
> +/* FEC XOR function *
> + *
> + * Compute XOR between two packets f1 and f2. New packet is stored in f1.
> + *
> + */
> +static void fec_xor(access_t *access, block_t *f1, const block_t *f2)
> +{
> + /* Check if f1 is still empty and needs initiailization */
> + if (f1->i_buffer == 0) {
> + memset(f1->p_buffer, 0, sizeof(struct dozer_hdr));
> + f1->i_buffer = f2->i_buffer;
> + memcpy(f1->p_buffer + HDR_EXCLUDE, f2->p_buffer + HDR_EXCLUDE, f1->i_buffer - HDR_EXCLUDE);
> + }
> + else if (f1->i_buffer != f2->i_buffer) {
> + msg_Warn(access, "XOR operation failed: buffer size mismatch.\n");
> + }
> + else if (f1->i_buffer < sizeof(struct dozer_hdr)) {
> + msg_Warn(access, "XOR operation failed: buffer size too short.\n");
> + } else {
> + for(size_t i = HDR_EXCLUDE; i < f1->i_buffer; i++)
> + f1->p_buffer[i] ^= f2->p_buffer[i];
> + }
> +}
> +
> +/*****************************************************************************
> + * Open: open the socket
> + *****************************************************************************/
> +static int Open( vlc_object_t *p_this )
> +{
> + access_t *p_access = (access_t*)p_this;
> + access_sys_t *sys;
> + char *psz_parse = NULL;
> +
> + if( p_access->b_preparsing ) {
> + msg_Err(p_access, "Cannot pre-parse dozer url.");
> + return VLC_EGENERIC;
> + }
> +
> + sys = calloc(1, sizeof( *sys ));
> + if( unlikely( sys == NULL ) ) {
> + msg_Err(p_access, "Out of Memory.");
> + return VLC_ENOMEM;
> + }
> +
> + p_access->p_sys = sys;
> + if (asprintf(&psz_parse, "udp://%s", p_access->psz_location) < 0) {
> + free(sys);
> + return VLC_ENOMEM;
> + }
> +
> + /* Set up p_access */
> + ACCESS_SET_CALLBACKS( NULL, BlockUDP, Control, NULL );
> +
> + /* Parse p_access->psz_location syntax :
> + * [serveraddr[:serverport]][@[bindaddr]:[bindport]] */
> + vlc_url_t url;
> + int res = vlc_UrlParse(&url, psz_parse);
> + if( unlikely(res < 0) ) {
> + msg_Err(p_access, "Cannot parse dozer url.");
> + return VLC_EGENERIC;
> + }
There are ways of using `vlc_UrlParse` in order to support your needs
without having to actually change the definition of `vlc_UrlParse`.
> +
> + int i_bind_port = url.i_bind_port > 0 ? url.i_bind_port : 1234;
> + const char *psz_bind_addr = url.psz_bind_host ? url.psz_bind_host : "";
> +
> + msg_Dbg( p_access, "opening server=%s:%d local=%s:%d",
> + url.psz_host, url.i_port, psz_bind_addr, i_bind_port );
> +
> + sys->fd = net_OpenDgram( p_access, psz_bind_addr, i_bind_port,
> + url.psz_host, url.i_port, IPPROTO_UDP );
> + if( sys->fd == -1 )
> + {
> + msg_Err( p_access, "cannot open socket" );
> + goto error;
> + }
> +
> + /* Revert to blocking I/O */
> +#ifndef _WIN32
> + fcntl(sys->fd, F_SETFL, fcntl(sys->fd, F_GETFL) & ~O_NONBLOCK);
> +#else
> + ioctlsocket(sys->fd, FIONBIO, &(unsigned long){ 0 });
> +#endif
> +
> + sys->fifo = block_FifoNew();
> + if( unlikely( sys->fifo == NULL ) )
> + {
> + net_Close( sys->fd );
> + goto error;
> + }
> +
> + sys->mtu = 7 * TS_PACKET_SIZE + sizeof(struct dozer_hdr);
> + sys->fifo_size = var_CreateGetInteger( p_access, "udp-buffer");
> + vlc_sem_init( &sys->semaphore, 0 );
> + if (sys->fifo_size < 50000) {
> + msg_Err(p_access, "Fifo too small (%lu), increasing to 50000", sys->fifo_size);
The above probably makes more sense being a warning, than an error. If
the variable `udp-buffer` shall not contain a value smaller than `50
000`, please specify this in the module configuration.
> + sys->fifo_size = 50000;
> + }
> +
> + sys->timeout = var_CreateGetInteger( p_access, "udp-timeout");
> + atomic_init(&sys->timeout_reached, false);
> + if( sys->timeout > 0)
> + sys->timeout *= 1000;
> +
> + if( vlc_clone( &sys->thread, ThreadRead, p_access,
> + VLC_THREAD_PRIORITY_INPUT ) )
> + {
> + vlc_sem_destroy( &sys->semaphore );
> + block_FifoRelease( sys->fifo );
> + net_Close( sys->fd );
> +error:
> + vlc_UrlClean(&url);
> + free(psz_parse);
> + free( sys );
> + return VLC_EGENERIC;
> + }
> +
> + vlc_UrlClean(&url);
> + free(psz_parse);
> + return VLC_SUCCESS;
> +}
> +
> +/*****************************************************************************
> + * Close: free unused data structures
> + *****************************************************************************/
> +static void Close( vlc_object_t *p_this )
> +{
> + access_t *p_access = (access_t*)p_this;
> + access_sys_t *sys = p_access->p_sys;
> +
> + vlc_cancel( sys->thread );
> + vlc_join( sys->thread, NULL );
> + vlc_sem_destroy( &sys->semaphore );
> + block_FifoRelease( sys->fifo );
> + net_Close( sys->fd );
> + if (sys->matrix) {
> + for (uint16_t i = 0; i < sys->n_tot; i++) {
> + if (sys->matrix[i]) block_Release(sys->matrix[i]);
> + }
> + free(sys->matrix);
> + }
> + if (sys->fec_rows) {
> + for (uint16_t i = 0; i < sys->n_rows; i++) {
> + if (sys->fec_rows[i]) block_Release(sys->fec_rows[i]);
> + }
> + free(sys->fec_rows);
> + }
> + if (sys->fec_cols ) {
> + for (uint16_t i = 0; i < sys->n_cols; i++) {
> + if (sys->fec_cols[i]) block_Release(sys->fec_cols[i]);
> + }
> + free(sys->fec_cols);
> + }
> + free( sys );
> +}
> +
> +/*****************************************************************************
> + * Control:
> + *****************************************************************************/
> +static int Control( access_t *p_access, int i_query, va_list args )
> +{
> + bool *pb_bool;
> + int64_t *pi_64;
> +
> + switch( i_query )
> + {
> + case STREAM_CAN_SEEK:
> + case STREAM_CAN_FASTSEEK:
> + case STREAM_CAN_PAUSE:
> + case STREAM_CAN_CONTROL_PACE:
> + pb_bool = (bool*)va_arg( args, bool* );
The above cast to `bool*` is redundant, `va_arg( args, bool* )`
already yields a `bool*`.
> + *pb_bool = false;
> + break;
> +
> + case STREAM_GET_PTS_DELAY:
> + pi_64 = va_arg( args, int64_t * );
> + *pi_64 = INT64_C(1000)
> + * var_InheritInteger(p_access, "network-caching");
> + break;
> +
> + default:
> + return VLC_EGENERIC;
> + }
> + return VLC_SUCCESS;
> +}
> +
> +/*****************************************************************************
> + * BlockUDP:
> + *****************************************************************************/
> +static block_t *BlockUDP( access_t *p_access, bool *restrict eof )
> +{
> + access_sys_t *sys = p_access->p_sys;
> + block_t *block;
> + if (atomic_load(&sys->timeout_reached)) {
> + *eof = true;
> + return NULL;
> + }
> + vlc_sem_wait_i11e(&sys->semaphore);
> + vlc_fifo_Lock(sys->fifo);
> + block = vlc_fifo_DequeueUnlocked(sys->fifo);
> + vlc_fifo_Unlock(sys->fifo);
> + return block;
> +}
> +
> +static void fifo_enqueue(access_t *access, block_t *pkt)
> +{
> + access_sys_t *sys = access->p_sys;
> + /* Discard old buffers on overflow */
> + while (vlc_fifo_GetBytes(sys->fifo) + pkt->i_buffer > sys->fifo_size)
> + {
> + int canc = vlc_savecancel();
> + block_t *old = vlc_fifo_DequeueUnlocked(sys->fifo);
> + if (old) {
> + block_Release(old);
> + } else {
> + break;
> + }
> + vlc_restorecancel(canc);
> + }
> + pkt->p_buffer += sizeof(struct dozer_hdr);
> + pkt->i_buffer -= sizeof(struct dozer_hdr);
> + vlc_fifo_QueueUnlocked(sys->fifo, pkt);
> +}
> +
> +static int fec_found_and_recovered(access_t *access, block_t *newblk, uint16_t new_idx,
> + uint16_t win)
> +{
> + struct dozer_hdr *new_hdr = (struct dozer_hdr *)newblk->p_buffer;
> + access_sys_t *sys = access->p_sys;
> +
> + new_hdr->type = FEC_TYPE_DATA;
> + new_hdr->seq = htons(new_idx);
> + new_hdr->win = htons(win);
> + new_hdr->count = 0;
> +
> + if (unlikely(htons(new_hdr->size) != (newblk->i_buffer - sizeof(struct dozer_hdr)))) {
> + msg_Err(access, "Dozer Recovery FAILED. Invalid size %u for recovered packet.",
> + ntohs(new_hdr->size));
> +
> + block_Release(newblk);
> + return 0;
> + }
> + msg_Dbg(access, "Dozer Packet Recovered. Cur win: %u idx: %u buffer size: %lu size \
> + stored: %u", ntohs(win), new_idx, newblk->i_buffer, ntohs(new_hdr->size));
The type of `newblk->i_buffer` is `size_t`, so there is nothing that
guarantees that `%lu` is the correct specifier in order to print the
value. Use `%zu`.
> +
> + sys->matrix[new_idx] = newblk;
> + if ((new_idx == sys->fec_last_rx + 1U)) {
> + sys->fec_last_rx++;
> + }
> +
> + while (sys->fec_last_rx < (sys->n_tot - 1) && (sys->matrix[sys->fec_last_rx + 1])) {
> + sys->fec_last_rx++;
> + }
Is `sys->n_tot` really guaranteed to be *bigger-than* `0` at this point? If
not the expression will underflow, causing you to read data outside
the bounds of `sys->matrix`.
> + return 1;
> +}
> +
> +static block_t *fec_rebuild_packet(access_t *access, enum fec_recover_type type, uint16_t r, uint16_t c)
> +{
> + block_t *fec_pkt;
> + access_sys_t *sys = access->p_sys;
> + block_t *newblk;
> +
> + if (type == RECOVER_ROW)
> + fec_pkt = sys->fec_rows[r];
> + else
> + fec_pkt = sys->fec_cols[c];
> +
> + if (unlikely(!fec_pkt)) {
> + msg_Dbg(access, "Dozer rebuild: no FEC packet avail.\n");
> + return NULL;
> + }
> +
> + if (unlikely((r >= sys->n_rows) || (c >= sys->n_cols)))
> + return NULL;
> +
> + newblk = block_Alloc(sys->mtu);
> + if (!newblk)
> + return NULL;
> +
> + newblk->i_buffer = 0;
> + fec_xor(access, newblk, fec_pkt);
> +
> + if (type == RECOVER_ROW) {
> + for (uint16_t i = 0; i < sys->n_cols; i++) {
> + if (i != c) {
> + fec_xor(access, newblk, sys->matrix[r * sys->n_cols + i]);
> + }
> + }
> + } else {
> + for (uint16_t i = 0; i < sys->n_rows; i++) {
> + if (i != r) {
> + fec_xor(access, newblk, sys->matrix[i * sys->n_cols + c]);
> + }
> + }
> + }
> + return newblk;
> +}
> +
> +static void fec_try_recovery(access_t *access, block_t *pkt)
> +{
> + access_sys_t *sys = access->p_sys;
> + struct dozer_hdr *hdr = (struct dozer_hdr *)(pkt->p_buffer);
> + uint16_t pkt_win = ntohs(hdr->win);
> + uint16_t pkt_seq = ntohs(hdr->seq);
> + uint16_t pkt_type = ntohs(hdr->type);
Weird indentation above; simply remove the whitespace.
> + int idx;
> + int missing = 0;
> + int rec_r = -1, rec_c = -1;
> + int new_idx = -1;
> + block_t *newblk = NULL;
> +
Below is another sign of *code-duplication*, could be refactored (and
simplified).
> + if (likely(pkt_win == sys->fec_cur_win)) {
> + if (pkt_type == FEC_TYPE_FEC_COL) {
> + sys->fec_cols[pkt_seq] = pkt;
> + for (uint16_t i = 0; i < sys->n_rows; i++) {
> + idx = i * sys->n_cols + pkt_seq;
> + if (!sys->matrix[idx]) {
> + missing++;
> + rec_r = i;
> + new_idx = idx;
> + }
> + }
> + if (missing == 1)
> + newblk = fec_rebuild_packet(access, RECOVER_COL, rec_r, pkt_seq);
> + } else if (pkt_type == FEC_TYPE_FEC_ROW) {
> + sys->fec_rows[pkt_seq] = pkt;
> + for (uint16_t i = 0; i < sys->n_cols; i++) {
> + idx = sys->n_cols * pkt_seq + i;
> + if (!sys->matrix[idx]) {
> + missing++;
> + rec_c = i;
> + new_idx = idx;
> + }
> + }
> + if (missing == 1)
> + newblk = fec_rebuild_packet(access, RECOVER_ROW, pkt_seq, rec_c);
> + }
> + if (newblk) {
> + fec_found_and_recovered(access, newblk, new_idx, ntohs(hdr->win));
> + return;
> + }
> + }
> +}
> +
> +static void process_fec_initials(access_t *access, block_t *pkt)
> +{
> + access_sys_t *sys = access->p_sys;
> + struct dozer_hdr *hdr = (struct dozer_hdr *)(pkt->p_buffer);
> + uint16_t pkt_type = ntohs(hdr->type);
> + uint16_t pkt_count = ntohs(hdr->count);
> +
> + /* Reject invalid values for rows or columns */
> + if ((pkt_count == 0) || (pkt_count > FEC_MATRIX_MAX))
> + return;
> +
> + if (pkt_type == FEC_TYPE_FEC_COL) {
> + sys->n_cols = pkt_count;
> + }
> + if (pkt_type == FEC_TYPE_FEC_ROW) {
> + sys->n_rows = pkt_count;
> + }
The usage of `{` and `}` in places such as the above does more harm
than good.
> + if ((sys->n_cols > 0) && (sys->n_rows > 0)) {
> + sys->n_tot = sys->n_cols * sys->n_rows;
> + sys->matrix = calloc(sys->n_tot, sizeof(block_t*));
> + sys->fec_rows = calloc(sys->n_rows, sizeof(block_t *));
> + sys->fec_cols = calloc(sys->n_cols, sizeof(block_t *));
> + sys->fec_initialized = 1;
> + fec_try_recovery(access, pkt);
> + } else {
> + block_Release(pkt);
> + }
> +}
> +
> +static int recover_frame(access_t *access, uint16_t idx)
> +{
> + uint16_t r, c;
> + int can_recover;
> + access_sys_t *sys = access->p_sys;
> + block_t *newblk = NULL;
> +
> + r = idx / sys->n_cols;
> + c = idx % sys->n_cols;
> +
> + can_recover = 0;
> + if (sys->fec_cols[c] != NULL) {
> + can_recover = 1;
> + for (uint16_t i = 0; i < sys->n_rows; i++) {
> + uint16_t cur = i * sys->n_cols + c;
> + if (((sys->matrix[cur]) == NULL) && (cur != idx)) {
> + /* Cannot recover: packet missing */
> + can_recover = 0;
> + break;
> + }
> + }
> + if (can_recover) {
> + newblk = fec_rebuild_packet(access, RECOVER_COL, r, c);
> + if (!newblk)
> + return 0;
> + }
> + }
The above *vs* the below; pretty much identical implementation, could
be refactored.
> + can_recover = 0;
> + if (sys->fec_rows[r] != NULL) {
> + can_recover = 1;
> + for (uint16_t i = 0; i < sys->n_cols; i++) {
> + uint16_t cur = r * sys->n_cols + i;
> + if (((sys->matrix[cur]) == NULL) && (cur != idx)) {
> + /* Cannot recover: packet missing */
> + can_recover = 0;
> + break;
> + }
> + }
> + if (can_recover) {
> + newblk = fec_rebuild_packet(access, RECOVER_ROW, r, c);
> + if (!newblk)
> + return 0;
> + }
> + }
> +
> + if (newblk)
> + return fec_found_and_recovered(access, newblk, idx, sys->fec_cur_win);
> + else
> + return 0;
> +}
> +
> +static void flush_win(access_t *access)
> +{
> + int frame_recovered;
> + access_sys_t *sys = access->p_sys;
> +
> + if (!sys->matrix)
> + return;
> + do {
> + frame_recovered = 0;
> + for (uint16_t i = 0; i < sys->n_tot; i++) {
> + if (!sys->matrix[i]) {
> + frame_recovered = recover_frame(access, i);
> + }
> + }
> + } while (frame_recovered > 0);
> +
> + vlc_fifo_Lock(sys->fifo);
> + for (uint16_t i = 0; i < sys->n_tot; i++) {
> + if (sys->matrix[i]) {
> + fifo_enqueue(access, sys->matrix[i]);
> + vlc_sem_post(&sys->semaphore);
> + sys->matrix[i] = NULL;
> + } else {
> + msg_Warn(access, "Warning: Dozer frame %u definitely missing!", i);
> + }
> + }
> + for (uint16_t i = 0; i < sys->n_rows; i++) {
> + if (sys->fec_rows[i]) {
> + block_Release(sys->fec_rows[i]);
> + sys->fec_rows[i] = NULL;
> + }
> + }
> + for (uint16_t i = 0; i < sys->n_cols; i++) {
> + if (sys->fec_cols[i]) {
> + block_Release(sys->fec_cols[i]);
> + sys->fec_cols[i] = NULL;
> + }
> + }
> + vlc_fifo_Unlock(sys->fifo);
> +}
> +
> +static int frame_to_matrix(access_t *access, block_t *pkt)
> +{
> + access_sys_t *sys = access->p_sys;
> + struct dozer_hdr *hdr = (struct dozer_hdr *)(pkt->p_buffer);
> + uint16_t win;
> + uint16_t idx;
> +
> + idx = ntohs(hdr->seq);
> + win = ntohs(hdr->win);
> +
> + if (idx > sys->n_tot)
> + return -1;
> +
> + if (sys->fec_cur_win != win) {
> + if (((uint16_t)(sys->fec_cur_win - win)) > 3) {
> + /* Next win started. */
> + flush_win(access);
> + sys->fec_cur_win = win;
> + sys->fec_last_rx = 0;
> + sys->matrix[idx] = pkt;
> + } else {
> + /* Probably old frame. Forward. */
> + msg_Dbg(access, "Received old? win: %d cur: %d", win, sys->fec_cur_win);
> + vlc_fifo_Lock(sys->fifo);
> + fifo_enqueue(access, pkt);
> + vlc_sem_post(&sys->semaphore);
> + vlc_fifo_Unlock(sys->fifo);
> + }
> + } else {
> + sys->matrix[idx] = pkt;
> + if (sys->fec_last_rx + 1 == idx) {
> + sys->fec_last_rx = idx;
> + while ((sys->fec_last_rx < (sys->n_tot - 1)) &&
> + (sys->matrix[sys->fec_last_rx + 1] != NULL)) {
> + sys->fec_last_rx++;
> + }
> + return idx;
> + }
> + }
> + return idx;
> +}
> +
> +static void process_fec(access_t *access, block_t *pkt)
> +{
> + access_sys_t *sys = access->p_sys;
> + struct dozer_hdr *hdr = (struct dozer_hdr *)(pkt->p_buffer);
> + uint16_t pkt_type = ntohs(hdr->type);
> + uint16_t pkt_size = ntohs(hdr->size);
> + uint16_t pkt_sig = ntohs(hdr->signature);
> +
> + /* Check signature */
> + if (pkt_sig != SIGNATURE) {
> + msg_Dbg(access, "Received invalid packet with sig: %04x. Discarding.",
> + pkt_sig);
> + block_Release(pkt);
> + return;
> + }
> +
> + if ((pkt_type == FEC_TYPE_FEC_COL) || (pkt_type == FEC_TYPE_FEC_ROW)) {
> + if (!sys->fec_initialized)
> + process_fec_initials(access, pkt);
> + else
> + fec_try_recovery(access, pkt);
> + } else if (pkt_type == FEC_TYPE_DATA) {
> + if (pkt_size != (uint16_t)(pkt->i_buffer - (sizeof(struct dozer_hdr)))) {
> + msg_Dbg(access, "Received invalid packet with wrong size: %u \
> + (expected %lu). Discarding.",
> + pkt_size, pkt->i_buffer);
> + block_Release(pkt);
> + return;
> + }
> + if (!sys->fec_initialized)
> + block_Release(pkt);
> + else
> + frame_to_matrix(access, pkt);
> + } else {
> + msg_Dbg(access, "Received invalid packet with type: %04x. Discarding.",
> + pkt_type);
> + block_Release(pkt);
> + }
> +}
> +
> +/*****************************************************************************
> + * ThreadRead: Pull packets from socket as soon as possible.
> + *****************************************************************************/
> +static void* ThreadRead( void *data )
> +{
> + access_t *access = data;
> + access_sys_t *sys = access->p_sys;
> +
> + for(;;)
> + {
> + block_t *pkt = block_Alloc(sys->mtu);
> + if (unlikely(pkt == NULL))
> + { /* OOM - dequeue and discard one packet */
> + char dummy;
> + recv(sys->fd, &dummy, 1, 0);
> + continue;
> + }
> +
> + struct iovec iov = {
> + .iov_base = pkt->p_buffer,
> + .iov_len = sys->mtu,
> + };
> + struct msghdr msg = {
> + .msg_iov = &iov,
> + .msg_iovlen = 1,
> +#ifdef MSG_TRUNC
> + .msg_flags = MSG_TRUNC,
> +#endif
> + };
> + ssize_t len;
> + block_cleanup_push(pkt);
> + do
> + {
> + int poll_return=0;
> + struct pollfd ufd[1];
> + ufd[0].fd = sys->fd;
> + ufd[0].events = POLLIN;
> +
> + /* cancellation point */
> + while ((poll_return = poll(ufd, 1, sys->timeout)) < 0) {
> + /* do nothing */;
> + }
> +
> + if (unlikely( poll_return == 0))
> + {
> + msg_Err( access, "Timeout on receiving, timeout %d seconds",
> + sys->timeout/1000 );
> + atomic_store(&sys->timeout_reached, true);
> + vlc_sem_post(&sys->semaphore);
> + len=0;
> + break;
> + }
> + len = recvmsg(sys->fd, &msg, 0);
> + }
> + while (len == -1);
> + vlc_cleanup_pop();
> +
> +#ifdef MSG_TRUNC
> + if (msg.msg_flags & MSG_TRUNC)
> + {
> + msg_Err(access, "%zd bytes packet truncated (MTU was %zu)",
> + len, sys->mtu);
> + pkt->i_flags |= BLOCK_FLAG_CORRUPTED;
> + sys->mtu = len;
> + }
> + else
> +#endif
> + pkt->i_buffer = len;
> +
> + process_fec(access, pkt);
> + }
> + return NULL;
> +}
> --
> 2.8.1
>
> From 0dc119af795dfae9472d9fe9fd5a2f88e2c2a104 Mon Sep 17 00:00:00 2001
> From: Daniele Lacamera <root at danielinux.net>
> Date: Sun, 30 Oct 2016 13:29:40 +0100
> Subject: [PATCH 4/6] NEWS: Announce new dozer streaming protocol
>
> ---
> NEWS | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index 5fa0cf6..a5cd3a9 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,8 @@ Core:
> * Refactor preparsing input
>
> Access:
> + * New Dozer input and output modules for tranmission of udp streams over lossy
> + networks.
*"tranmission"* `=>` *"transmission"*
> * New NFS access module using libnfs
> * New SMB access module using libdsm
> * Rewrite MPEG-DASH (Dynamic Adaptive Streaming over HTTP) support, including
> --
> 2.8.1
>
> From 3e8242da92ede75b91173b7d9fe5a661170351c6 Mon Sep 17 00:00:00 2001
> From: Jean-Paul Saman <jpsaman at videolan.org>
> Date: Sun, 30 Oct 2016 13:28:02 +0100
> Subject: [PATCH 3/6] udp: add udp:// to p_access->psz_location
>
> ---
> modules/access/udp.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/modules/access/udp.c b/modules/access/udp.c
> index c156ae7..5f50dc5 100644
> --- a/modules/access/udp.c
> +++ b/modules/access/udp.c
> @@ -107,12 +107,18 @@ static int Open( vlc_object_t *p_this )
> /* Set up p_access */
> ACCESS_SET_CALLBACKS( NULL, BlockUDP, Control, NULL );
>
> + char *psz_parse = NULL;
> + if (asprintf(&psz_parse, "udp://%s", p_access->psz_location) < 0) {
> + free(sys);
> + return VLC_ENOMEM;
> + }
> +
> /* Parse p_access->psz_location syntax :
> * [serveraddr[:serverport]][@[bindaddr]:[bindport]] */
> vlc_url_t url;
> - int res = vlc_UrlParse(&url, p_access->psz_location);
> + int res = vlc_UrlParse(&url, psz_parse);
> if( unlikely(res < 0) )
> - goto error;
> + return VLC_EGENERIC;
>
> int i_bind_port = url.i_bind_port > 0 ? url.i_bind_port : 1234;
> const char *psz_bind_addr = url.psz_bind_host ? url.psz_bind_host : "";
> @@ -122,13 +128,12 @@ static int Open( vlc_object_t *p_this )
>
> sys->fd = net_OpenDgram( p_access, psz_bind_addr, i_bind_port,
> url.psz_host, url.i_port, IPPROTO_UDP );
> - vlc_UrlClean(&url);
> -
> if( sys->fd == -1 )
> {
> msg_Err( p_access, "cannot open socket" );
> -error:
> free( sys );
> + vlc_UrlClean(&url);
> + free(psz_parse);
> return VLC_EGENERIC;
> }
>
> @@ -138,6 +143,8 @@ error:
> if( sys->timeout > 0)
> sys->timeout *= 1000;
>
> + vlc_UrlClean(&url);
> + free(psz_parse);
> return VLC_SUCCESS;
> }
>
> --
> 2.8.1
>
> From ebbfe4c13e2009921d4867a458a2e96818d6e499 Mon Sep 17 00:00:00 2001
> From: Jean-Paul Saman <jpsaman at videolan.org>
> Date: Thu, 27 Oct 2016 15:42:26 +0200
> Subject: [PATCH 2/6] access/udp.c: use vlc_UrlParse()
>
> ---
> modules/access/udp.c | 59 +++++++++++-----------------------------------------
> 1 file changed, 12 insertions(+), 47 deletions(-)
>
> diff --git a/modules/access/udp.c b/modules/access/udp.c
> index ced4f8e..c156ae7 100644
> --- a/modules/access/udp.c
> +++ b/modules/access/udp.c
> @@ -42,6 +42,7 @@
> #include <vlc_access.h>
> #include <vlc_network.h>
> #include <vlc_block.h>
> +#include <vlc_url.h>
> #include <vlc_interrupt.h>
> #ifdef HAVE_POLL
> # include <poll.h>
> @@ -106,59 +107,23 @@ static int Open( vlc_object_t *p_this )
> /* Set up p_access */
> ACCESS_SET_CALLBACKS( NULL, BlockUDP, Control, NULL );
>
> - char *psz_name = strdup( p_access->psz_location );
> - char *psz_parser;
> - const char *psz_server_addr, *psz_bind_addr = "";
> - int i_bind_port = 1234, i_server_port = 0;
> -
> - if( unlikely(psz_name == NULL) )
> - goto error;
> -
> - /* Parse psz_name syntax :
> + /* Parse p_access->psz_location syntax :
> * [serveraddr[:serverport]][@[bindaddr]:[bindport]] */
> - psz_parser = strchr( psz_name, '@' );
> - if( psz_parser != NULL )
> - {
> - /* Found bind address and/or bind port */
> - *psz_parser++ = '\0';
> - psz_bind_addr = psz_parser;
> -
> - if( psz_bind_addr[0] == '[' )
> - /* skips bracket'd IPv6 address */
> - psz_parser = strchr( psz_parser, ']' );
> -
> - if( psz_parser != NULL )
> - {
> - psz_parser = strchr( psz_parser, ':' );
> - if( psz_parser != NULL )
> - {
> - *psz_parser++ = '\0';
> - i_bind_port = atoi( psz_parser );
> - }
> - }
> - }
> + vlc_url_t url;
> + int res = vlc_UrlParse(&url, p_access->psz_location);
> + if( unlikely(res < 0) )
> + goto error;
>
> - psz_server_addr = psz_name;
> - psz_parser = ( psz_server_addr[0] == '[' )
> - ? strchr( psz_name, ']' ) /* skips bracket'd IPv6 address */
> - : psz_name;
> -
> - if( psz_parser != NULL )
> - {
> - psz_parser = strchr( psz_parser, ':' );
> - if( psz_parser != NULL )
> - {
> - *psz_parser++ = '\0';
> - i_server_port = atoi( psz_parser );
> - }
> - }
> + int i_bind_port = url.i_bind_port > 0 ? url.i_bind_port : 1234;
> + const char *psz_bind_addr = url.psz_bind_host ? url.psz_bind_host : "";
>
> msg_Dbg( p_access, "opening server=%s:%d local=%s:%d",
> - psz_server_addr, i_server_port, psz_bind_addr, i_bind_port );
> + url.psz_host, url.i_port, psz_bind_addr, i_bind_port );
>
> sys->fd = net_OpenDgram( p_access, psz_bind_addr, i_bind_port,
> - psz_server_addr, i_server_port, IPPROTO_UDP );
> - free( psz_name );
> + url.psz_host, url.i_port, IPPROTO_UDP );
> + vlc_UrlClean(&url);
> +
> if( sys->fd == -1 )
> {
> msg_Err( p_access, "cannot open socket" );
> --
> 2.8.1
>
> From 829791d6a96f880927f5b9673e32f45da3090879 Mon Sep 17 00:00:00 2001
> From: Jean-Paul Saman <jpsaman at videolan.org>
> Date: Thu, 27 Oct 2016 15:42:24 +0200
> Subject: [PATCH 1/6] vlc_url: parse '@[bindhost]:[bindport]' in
> '[serveraddr[:serverport]][@[bindaddr]:[bindport]]'
>
> ---
> include/vlc_url.h | 2 ++
> src/text/url.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/include/vlc_url.h b/include/vlc_url.h
> index 5a20c27..802227e 100644
> --- a/include/vlc_url.h
> +++ b/include/vlc_url.h
> @@ -149,6 +149,8 @@ struct vlc_url_t
> char *psz_password;
> char *psz_host;
> unsigned i_port;
> + char *psz_bind_host;
> + unsigned i_bind_port;
> char *psz_path;
> char *psz_option;
>
> diff --git a/src/text/url.c b/src/text/url.c
> index b9e7b41..4999524 100644
> --- a/src/text/url.c
> +++ b/src/text/url.c
> @@ -408,6 +408,8 @@ int vlc_UrlParse(vlc_url_t *restrict url, const char *str)
> url->psz_password = NULL;
> url->psz_host = NULL;
> url->i_port = 0;
> + url->psz_bind_host = NULL;
> + url->i_bind_port = 0;
> url->psz_path = NULL;
> url->psz_option = NULL;
> url->psz_buffer = NULL;
> @@ -474,27 +476,78 @@ int vlc_UrlParse(vlc_url_t *restrict url, const char *str)
> /*else
> url->psz_path = "/";*/
>
> - /* User name */
> - next = strrchr(cur, '@');
> - if (next != NULL)
> + if (url->psz_protocol && (strcmp(url->psz_protocol, "udp") == 0))
> {
> - *(next++) = '\0';
> - url->psz_username = cur;
> - cur = next;
> -
> - /* Password (obsolete) */
> - next = strchr(url->psz_username, ':');
> + /*
> + * Parse udp syntax:
> + * [serveraddr[:serverport]][@[bindaddr]:[bindport]]
> + */
> + next = strchr(cur, '@');
> + if (next != NULL)
> + {
> + *(next++) = '\0';
> + char *psz_bindhost = strdup(next);
> + if (*psz_bindhost == '[' && (next = strchr(psz_bindhost, ']')) != NULL)
> + {
> + /* Try IPv6 numeral within brackets */
> + *(next++) = '\0';
> + url->psz_bind_host = strdup(psz_bindhost + 1);
> +
> + if (*next == ':')
> + next++;
> + else
> + next = NULL;
> + }
> + else
> + {
> + next = strchr(psz_bindhost, ':');
> + if (next != NULL)
> + *(next++) = '\0';
> +
> + url->psz_bind_host = vlc_idna_to_ascii(vlc_uri_decode(psz_bindhost));
> + }
> +
> + if (url->psz_bind_host == NULL)
> + ret = -1;
> + else
> + if (!vlc_uri_host_validate(url->psz_bind_host))
> + {
> + free(url->psz_bind_host);
> + url->psz_bind_host = NULL;
> + errno = EINVAL;
> + ret = -1;
> + }
> + /* Port number */
> + if (next != NULL)
> + url->i_bind_port = atoi(next);
> +
> + free(psz_bindhost);
> + }
> + }
> + else
> + {
> + /* User name */
> + next = strrchr(cur, '@');
> if (next != NULL)
> {
> *(next++) = '\0';
> - url->psz_password = next;
> - vlc_uri_decode(url->psz_password);
> + url->psz_username = cur;
> + cur = next;
> +
> + /* Password (obsolete) */
> + next = strchr(url->psz_username, ':');
> + if (next != NULL)
> + {
> + *(next++) = '\0';
> + url->psz_password = next;
> + vlc_uri_decode(url->psz_password);
> + }
> + vlc_uri_decode(url->psz_username);
> }
> - vlc_uri_decode(url->psz_username);
> }
>
> /* Host name */
> - if (*cur == '[' && (next = strrchr(cur, ']')) != NULL)
> + if (*cur == '[' && (next = strchr(cur, ']')) != NULL)
> { /* Try IPv6 numeral within brackets */
> *(next++) = '\0';
> url->psz_host = strdup(cur + 1);
> @@ -559,6 +612,7 @@ int vlc_UrlParse(vlc_url_t *restrict url, const char *str)
>
> void vlc_UrlClean (vlc_url_t *restrict url)
> {
> + free (url->psz_bind_host);
> free (url->psz_host);
> free (url->psz_buffer);
> }
> --
> 2.8.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161106/936a93dd/attachment-0001.html>
More information about the vlc-devel
mailing list