On Wed, Jun 01, 2016 at 11:19:38AM +0200, Vlad Tarca wrote: > Pro-MPEG Code of Practice #3 release 2 forward error correction for > rtp_mpegts streams > > Fixes: > > [prompeg.c] > - proper RTP size check to fit the largest buffer > - UDP port overflow check > - replaced ffurl_close calls with ffurl_closep > [rtpproto.c] > - removed context fec flag and used fec_hd directly > - moved fec_hd open outside the retry loop as it doesn't set any specific > local ports > - replaced ffurl_close call with ffurl_closep > > Signed-off-by: Vlad Tarca <vta...@mobibase.com>
please provide a commit message that works for the commit, not one
that lists th differences to the last patch
(as only the final one is commited that would be confusing)
[...]
> +static void xor_fast(const uint8_t *in1, const uint8_t *in2, uint8_t *out,
> int size) {
> + int i, n, s;
> +
> +#if HAVE_FAST_64BIT
> + uint64_t v1, v2;
> +
> + n = size / sizeof (uint64_t);
> + s = n * sizeof (uint64_t);
> +
> + for (i = 0; i < n; i++) {
> + v1 = AV_RN64A(in1);
> + v2 = AV_RN64A(in2);
> + AV_WN64A(out, v1 ^ v2);
> + in1 += 8;
> + in2 += 8;
> + out += 8;
> + }
> +#else
> + uint32_t v1, v2;
> +
> + n = size / sizeof (uint32_t);
> + s = n * sizeof (uint32_t);
> +
> + for (i = 0; i < n; i++) {
> + v1 = AV_RN32A(in1);
> + v2 = AV_RN32A(in2);
> + AV_WN32A(out, v1 ^ v2);
> + in1 += 4;
> + in2 += 4;
> + out += 4;
> + }
> +#endif
> +
> + if (s == size)
> + return;
is it faster with this check ?
iam asking because the check doesnt change the outcome in any way
[...]
> +static int prompeg_open(URLContext *h, const char *uri, int flags) {
> + PrompegContext *s = h->priv_data;
> + AVDictionary *udp_opts = NULL;
> + int rtp_port;
> + char hostname[256];
> + char buf[1024];
> +
> + s->fec_col_hd = NULL;
> + s->fec_row_hd = NULL;
> +
> + if (s->l * s->d > 100) {
> + av_log(h, AV_LOG_ERROR, "L * D must be <= 100\n");
> + return AVERROR(EINVAL);
> + }
> +
> + av_url_split(NULL, 0, NULL, 0, hostname, sizeof (hostname), &rtp_port,
> + NULL, 0, uri);
> +
> + if (rtp_port < 1 || rtp_port > 65531) {
> + av_log(h, AV_LOG_ERROR, "Invalid RTP base port %d\n", rtp_port);
> + return AVERROR(EINVAL);
> + }
> +
> + if (s->ttl > 0) {
> + snprintf(buf, sizeof (buf), "%d", s->ttl);
> + av_dict_set(&udp_opts, "ttl", buf, 0);
> + }
> +
> + ff_url_join(buf, sizeof (buf), "udp", NULL, hostname, rtp_port + 2,
> NULL);
> + if (ffurl_open_whitelist(&s->fec_col_hd, buf, flags,
> &h->interrupt_callback,
> + &udp_opts, h->protocol_whitelist, h->protocol_blacklist, h) < 0)
> + goto fail;
> + ff_url_join(buf, sizeof (buf), "udp", NULL, hostname, rtp_port + 4,
> NULL);
> + if (ffurl_open_whitelist(&s->fec_row_hd, buf, flags,
> &h->interrupt_callback,
> + &udp_opts, h->protocol_whitelist, h->protocol_blacklist, h) < 0)
> + goto fail;
> +
> + h->max_packet_size = s->fec_col_hd->max_packet_size;
> + s->init = 1;
> +
> + av_dict_free(&udp_opts);
> + av_log(h, AV_LOG_INFO, "ProMPEG CoP#3-R2 FEC L=%d D=%d\n", s->l, s->d);
> + return 0;
> +
> +fail:
> + ffurl_closep(&s->fec_col_hd);
> + ffurl_closep(&s->fec_row_hd);
> + av_dict_free(&udp_opts);
> + return AVERROR(EIO);
> +}
> +
> +static int prompeg_init(URLContext *h, const uint8_t *buf, int size) {
> + PrompegContext *s = h->priv_data;
> + uint32_t seed;
> + int bitstring_len, rtp_buf_len;
> + int i;
> +
> + s->fec_buf = NULL;
> + s->rtp_buf = NULL;
> +
> + if (size < 12 || size >= INT_MAX - 16) {
> + av_log(h, AV_LOG_ERROR, "Invalid RTP packet size\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + s->packet_idx = 0;
> + s->packet_idx_max = s->l * s->d;
> + s->packet_size = size;
> + s->recovery_len = size - 12;
> + rtp_buf_len = 28 + s->recovery_len; // 12 + 16: RTP + FEC headers
> + s->rtp_buf_size = rtp_buf_len * sizeof (uint8_t);
> + bitstring_len = 8 + s->recovery_len; // 8: P, X, CC, M, PT, SN, TS
> + s->bitstring_size = bitstring_len * sizeof (uint8_t);
sizeof (uint8_t) is always 1, the multiplications arent needed
also why are some called _size and some _len ?
[...]
> @@ -412,6 +441,14 @@ static int rtp_open(URLContext *h, const char *uri, int
> flags)
> break;
> }
>
> + s->fec_hd = NULL;
> + if (fec_protocol) {
> + ff_url_join(buf, sizeof(buf), fec_protocol, NULL, hostname,
> rtp_port, NULL);
> + if (ffurl_open_whitelist(&s->fec_hd, buf, flags,
> &h->interrupt_callback,
> + &fec_opts, h->protocol_whitelist,
> h->protocol_blacklist, h) < 0)
> + goto fail;
something is wrong with the indention here
[...]
> @@ -474,7 +518,7 @@ static int rtp_read(URLContext *h, uint8_t *buf, int size)
> static int rtp_write(URLContext *h, const uint8_t *buf, int size)
> {
> RTPContext *s = h->priv_data;
> - int ret;
> + int ret, ret_fec;
> URLContext *hd;
>
> if (size < 2)
> @@ -543,7 +587,18 @@ static int rtp_write(URLContext *h, const uint8_t *buf,
> int size)
> hd = s->rtp_hd;
> }
>
> - ret = ffurl_write(hd, buf, size);
> + if ((ret = ffurl_write(hd, buf, size)) < 0) {
> + goto end;
> + }
> +
> + if (s->fec_hd && !RTP_PT_IS_RTCP(buf[1])) {
> + if ((ret_fec = ffurl_write(s->fec_hd, buf, size)) < 0) {
> + av_log(h, AV_LOG_ERROR, "Failed to send FEC\n");
> + ret = ret_fec;
> + }
> + }
> +
> +end:
> return ret;
the goto isnt needed, a direct return can be used
also is ret_fec needed and cant ret be used directly ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
