diff mbox series

[FFmpeg-devel] libavformat: add librist protocol

Message ID 20210228195655.11778-1-onemda@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] libavformat: add librist protocol | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Paul B Mahol Feb. 28, 2021, 7:56 p.m. UTC
This work is sponsored by Open Broadcast Systems.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 configure               |   5 +
 doc/protocols.texi      |  29 +++++
 libavformat/Makefile    |   1 +
 libavformat/librist.c   | 236 ++++++++++++++++++++++++++++++++++++++++
 libavformat/protocols.c |   1 +
 5 files changed, 272 insertions(+)
 create mode 100644 libavformat/librist.c

Comments

Marton Balint Feb. 28, 2021, 10:13 p.m. UTC | #1
On Sun, 28 Feb 2021, Paul B Mahol wrote:

> This work is sponsored by Open Broadcast Systems.
>
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
> configure               |   5 +
> doc/protocols.texi      |  29 +++++
> libavformat/Makefile    |   1 +
> libavformat/librist.c   | 236 ++++++++++++++++++++++++++++++++++++++++
> libavformat/protocols.c |   1 +
> 5 files changed, 272 insertions(+)
> create mode 100644 libavformat/librist.c
>

[...]

> +static int librist_read(URLContext *h, uint8_t *buf, int size)
> +{
> +    RISTContext *s = h->priv_data;
> +    const struct rist_data_block *data_block;
> +    int ret;
> +
> +    ret = rist_receiver_data_read(s->ctx, &data_block, s->queue_count <= 0 ? POLLING_TIME : 0);

Use POLLING_TIME unconditionally. If there are packets in the queue, 
POLLING_TIME should not matter. This also means that keeping track of 
queue_count is useless.

> +    if (ret < 0)
> +        return risterr2ret(ret);
> +
> +    if (ret == 0 || data_block->payload_len <= 0)
> +        return 0;

You should return EAGAIN if ret == 0, as in the previous version of the 
patch. Otherwise you are "making up" a 0 sized packet.

Yet again, this should not make a difference, because 
retry_transfer_wrapper in libavformat/avio.c fast retries EAGAIN 
immedietly for the first five times. So the user will only see an EAGAIN 
if the queue was empty for 5 * POLLING_TIME, which is 0.5 sec. If a 
librist stream receives no packets for 0.5 secs, then something is wrong.

Regards,
Marton
Paul B Mahol Feb. 28, 2021, 10:17 p.m. UTC | #2
On Sun, Feb 28, 2021 at 11:14 PM Marton Balint <cus@passwd.hu> wrote:

>
>
> On Sun, 28 Feb 2021, Paul B Mahol wrote:
>
> > This work is sponsored by Open Broadcast Systems.
> >
> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > ---
> > configure               |   5 +
> > doc/protocols.texi      |  29 +++++
> > libavformat/Makefile    |   1 +
> > libavformat/librist.c   | 236 ++++++++++++++++++++++++++++++++++++++++
> > libavformat/protocols.c |   1 +
> > 5 files changed, 272 insertions(+)
> > create mode 100644 libavformat/librist.c
> >
>
> [...]
>
> > +static int librist_read(URLContext *h, uint8_t *buf, int size)
> > +{
> > +    RISTContext *s = h->priv_data;
> > +    const struct rist_data_block *data_block;
> > +    int ret;
> > +
> > +    ret = rist_receiver_data_read(s->ctx, &data_block, s->queue_count
> <= 0 ? POLLING_TIME : 0);
>
> Use POLLING_TIME unconditionally. If there are packets in the queue,
> POLLING_TIME should not matter. This also means that keeping track of
> queue_count is useless.
>


Not possible, that would cause fifo filling up.

I give up. FFmpeg protocols API is utterly useless.


>
> > +    if (ret < 0)
> > +        return risterr2ret(ret);
> > +
> > +    if (ret == 0 || data_block->payload_len <= 0)
> > +        return 0;
>
> You should return EAGAIN if ret == 0, as in the previous version of the
> patch. Otherwise you are "making up" a 0 sized packet.
>
> Yet again, this should not make a difference, because
> retry_transfer_wrapper in libavformat/avio.c fast retries EAGAIN
> immedietly for the first five times. So the user will only see an EAGAIN
> if the queue was empty for 5 * POLLING_TIME, which is 0.5 sec. If a
> librist stream receives no packets for 0.5 secs, then something is wrong.
>
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint Feb. 28, 2021, 10:33 p.m. UTC | #3
On Sun, 28 Feb 2021, Paul B Mahol wrote:

> On Sun, Feb 28, 2021 at 11:14 PM Marton Balint <cus@passwd.hu> wrote:
>
>>
>>
>> On Sun, 28 Feb 2021, Paul B Mahol wrote:
>>
>> > This work is sponsored by Open Broadcast Systems.
>> >
>> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> > ---
>> > configure               |   5 +
>> > doc/protocols.texi      |  29 +++++
>> > libavformat/Makefile    |   1 +
>> > libavformat/librist.c   | 236 ++++++++++++++++++++++++++++++++++++++++
>> > libavformat/protocols.c |   1 +
>> > 5 files changed, 272 insertions(+)
>> > create mode 100644 libavformat/librist.c
>> >
>>
>> [...]
>>
>> > +static int librist_read(URLContext *h, uint8_t *buf, int size)
>> > +{
>> > +    RISTContext *s = h->priv_data;
>> > +    const struct rist_data_block *data_block;
>> > +    int ret;
>> > +
>> > +    ret = rist_receiver_data_read(s->ctx, &data_block, s->queue_count
>> <= 0 ? POLLING_TIME : 0);
>>
>> Use POLLING_TIME unconditionally. If there are packets in the queue,
>> POLLING_TIME should not matter. This also means that keeping track of
>> queue_count is useless.
>>
>
>
> Not possible, that would cause fifo filling up.

But why?

Here is the code directly from librist:

ssize_t num = 0;
// Select the flow with highest queue count to minimize jitter for calling app
struct rist_flow *f = rist_get_longest_flow(ctx, &num);
if (!num && timeout > 0)
{
         pthread_mutex_lock(&(ctx->mutex));
         pthread_cond_timedwait_ms(&(ctx->condition), &(ctx->mutex), timeout);
         pthread_mutex_unlock(&(ctx->mutex));
         f = rist_get_longest_flow(ctx, &num);
}

timeout should only matter, if queue is empty.

>
> I give up. FFmpeg protocols API is utterly useless.

Please, don't, if using a constant POLLING_TIME indeed makes a difference 
then you are on the right track of finding the core bug.

Regards,
Marton
diff mbox series

Patch

diff --git a/configure b/configure
index d11942fced..378b1e0cc2 100755
--- a/configure
+++ b/configure
@@ -259,6 +259,7 @@  External library support:
   --enable-libpulse        enable Pulseaudio input via libpulse [no]
   --enable-librabbitmq     enable RabbitMQ library [no]
   --enable-librav1e        enable AV1 encoding via rav1e [no]
+  --enable-librist         enable RIST via librist [no]
   --enable-librsvg         enable SVG rasterization via librsvg [no]
   --enable-librubberband   enable rubberband needed for rubberband filter [no]
   --enable-librtmp         enable RTMP[E] support via librtmp [no]
@@ -1797,6 +1798,7 @@  EXTERNAL_LIBRARY_LIST="
     libpulse
     librabbitmq
     librav1e
+    librist
     librsvg
     librtmp
     libshine
@@ -3490,6 +3492,8 @@  unix_protocol_select="network"
 # external library protocols
 libamqp_protocol_deps="librabbitmq"
 libamqp_protocol_select="network"
+librist_protocol_deps="librist"
+librist_protocol_select="network"
 librtmp_protocol_deps="librtmp"
 librtmpe_protocol_deps="librtmp"
 librtmps_protocol_deps="librtmp"
@@ -6409,6 +6413,7 @@  enabled libopus           && {
 enabled libpulse          && require_pkg_config libpulse libpulse pulse/pulseaudio.h pa_context_new
 enabled librabbitmq       && require_pkg_config librabbitmq "librabbitmq >= 0.7.1" amqp.h amqp_new_connection
 enabled librav1e          && require_pkg_config librav1e "rav1e >= 0.4.0" rav1e.h rav1e_context_new
+enabled librist           && require_pkg_config librist "librist >= 0.2" librist/librist.h rist_receiver_create
 enabled librsvg           && require_pkg_config librsvg librsvg-2.0 librsvg-2.0/librsvg/rsvg.h rsvg_handle_render_cairo
 enabled librtmp           && require_pkg_config librtmp librtmp librtmp/rtmp.h RTMP_Socket
 enabled librubberband     && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new -lstdc++ && append librubberband_extralibs "-lstdc++"
diff --git a/doc/protocols.texi b/doc/protocols.texi
index c0b511b7a4..11fb351bf6 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -690,6 +690,35 @@  Example usage:
 -f rtp_mpegts -fec prompeg=l=8:d=4 rtp://@var{hostname}:@var{port}
 @end example
 
+@section rist
+
+Reliable Internet Streaming Transport protocol
+
+The accepted options are:
+@table @option
+@item rist_profile
+Supported values:
+@table @samp
+@item simple
+@item main
+This one is default.
+@item advanced
+@end table
+
+@item buffer_size
+Set internal RIST buffer size for retransmission of data.
+
+@item log_level
+Set loglevel for RIST logging messages.
+
+@item secret
+Set override of encryption secret, by default is unset.
+
+@item encryption
+Set encryption type, by default is disabled.
+Acceptable values are 128 and 256.
+@end table
+
 @section rtmp
 
 Real-Time Messaging Protocol.
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 56fa96ea7f..95ed25e866 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -654,6 +654,7 @@  OBJS-$(CONFIG_UNIX_PROTOCOL)             += unix.o
 
 # external library protocols
 OBJS-$(CONFIG_LIBAMQP_PROTOCOL)          += libamqp.o
+OBJS-$(CONFIG_LIBRIST_PROTOCOL)          += librist.o
 OBJS-$(CONFIG_LIBRTMP_PROTOCOL)          += librtmp.o
 OBJS-$(CONFIG_LIBRTMPE_PROTOCOL)         += librtmp.o
 OBJS-$(CONFIG_LIBRTMPS_PROTOCOL)         += librtmp.o
diff --git a/libavformat/librist.c b/libavformat/librist.c
new file mode 100644
index 0000000000..789cefda36
--- /dev/null
+++ b/libavformat/librist.c
@@ -0,0 +1,236 @@ 
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Reliable Internet Streaming Transport protocol
+ */
+
+#include "libavutil/avassert.h"
+#include "libavutil/opt.h"
+#include "libavutil/parseutils.h"
+#include "libavutil/time.h"
+
+#include "avformat.h"
+#include "internal.h"
+#include "network.h"
+#include "os_support.h"
+#include "url.h"
+
+#include <librist/librist.h>
+
+typedef struct RISTContext {
+    const AVClass *class;
+
+    int queue_count;
+    int profile;
+    int buffer_size;
+    int log_level;
+    int encryption;
+    char *secret;
+
+    struct rist_logging_settings logging_settings;
+    struct rist_peer_config peer_config;
+
+    struct rist_peer *peer;
+    struct rist_ctx *ctx;
+} RISTContext;
+
+#define D AV_OPT_FLAG_DECODING_PARAM
+#define E AV_OPT_FLAG_ENCODING_PARAM
+#define OFFSET(x) offsetof(RISTContext, x)
+static const AVOption librist_options[] = {
+    { "rist_profile","set profile",     OFFSET(profile),     AV_OPT_TYPE_INT,   {.i64=RIST_PROFILE_MAIN},     0, 2, .flags = D|E, "profile" },
+    { "simple",      NULL,              0,                   AV_OPT_TYPE_CONST, {.i64=RIST_PROFILE_SIMPLE},   0, 0, .flags = D|E, "profile" },
+    { "main",        NULL,              0,                   AV_OPT_TYPE_CONST, {.i64=RIST_PROFILE_MAIN},     0, 0, .flags = D|E, "profile" },
+    { "advanced",    NULL,              0,                   AV_OPT_TYPE_CONST, {.i64=RIST_PROFILE_ADVANCED}, 0, 0, .flags = D|E, "profile" },
+    { "buffer_size", "set buffer_size", OFFSET(buffer_size), AV_OPT_TYPE_INT,   {.i64=0},                     0, INT_MAX, .flags = D|E },
+    { "log_level",   "set loglevel",    OFFSET(log_level),   AV_OPT_TYPE_INT,   {.i64=-1},                   -1, INT_MAX, .flags = D|E },
+    { "secret", "set encryption secret",OFFSET(secret),      AV_OPT_TYPE_STRING,{.str=NULL},                  0, 0,       .flags = D|E },
+    { "encryption","set encryption type",OFFSET(encryption), AV_OPT_TYPE_INT   ,{.i64=0},                     0, INT_MAX, .flags = D|E },
+    { NULL }
+};
+
+static int risterr2ret(int err)
+{
+    switch (err) {
+    case RIST_ERR_MALLOC:
+        return AVERROR(ENOMEM);
+    default:
+        return AVERROR_EXTERNAL;
+    }
+}
+
+static int log_cb(void *arg, enum rist_log_level log_level, const char *msg)
+{
+    int level;
+
+    switch (log_level) {
+    case RIST_LOG_ERROR:    level = AV_LOG_ERROR;   break;
+    case RIST_LOG_WARN:     level = AV_LOG_WARNING; break;
+    case RIST_LOG_NOTICE:   level = AV_LOG_VERBOSE; break;
+    case RIST_LOG_INFO:     level = AV_LOG_INFO;    break;
+    case RIST_LOG_DEBUG:    level = AV_LOG_DEBUG;   break;
+    case RIST_LOG_DISABLE:  level = AV_LOG_QUIET;   break;
+    case RIST_LOG_SIMULATE: level = AV_LOG_TRACE;   break;
+    default: level = AV_LOG_PANIC;
+    }
+
+    av_log(arg, level, "%s", msg);
+
+    return 0;
+}
+
+static int librist_close(URLContext *h)
+{
+    RISTContext *s = h->priv_data;
+    int ret = 0;
+
+    s->peer = NULL;
+
+    if (s->ctx)
+        ret = rist_destroy(s->ctx);
+    s->ctx = NULL;
+
+    return risterr2ret(ret);
+}
+
+static int librist_open(URLContext *h, const char *uri, int flags)
+{
+    RISTContext *s = h->priv_data;
+    struct rist_logging_settings *logging_settings = &s->logging_settings;
+    struct rist_peer_config *peer_config = &s->peer_config;
+    int ret;
+
+    if ((flags & AVIO_FLAG_READ_WRITE) == AVIO_FLAG_READ_WRITE)
+        return AVERROR(EINVAL);
+
+    ret = rist_logging_set(&logging_settings, s->log_level, log_cb, h, NULL, NULL);
+    if (ret < 0)
+        return risterr2ret(ret);
+
+    if (flags & AVIO_FLAG_WRITE)
+        ret = rist_sender_create(&s->ctx, s->profile, 0, logging_settings);
+    if (ret < 0)
+        goto err;
+
+    if (flags & AVIO_FLAG_READ)
+        ret = rist_receiver_create(&s->ctx, s->profile, logging_settings);
+    if (ret < 0)
+        goto err;
+
+    ret = rist_peer_config_defaults_set(peer_config);
+    if (ret < 0)
+        goto err;
+
+    ret = rist_parse_address(uri, (const struct rist_peer_config **)&peer_config);
+    if (ret < 0)
+        goto err;
+
+    if (((s->encryption == 128 || s->encryption == 256) && !s->secret) ||
+        ((peer_config->key_size == 128 || peer_config->key_size == 256) && !peer_config->secret[0])) {
+        av_log(h, AV_LOG_ERROR, "secret is mandatory if encryption is enabled\n");
+        librist_close(h);
+        return AVERROR(EINVAL);
+    }
+
+    if (s->secret && peer_config->secret[0] == 0)
+        av_strlcpy(peer_config->secret, s->secret, FFMIN(RIST_MAX_STRING_SHORT - 1, strlen(s->secret)));
+
+    if (s->secret && (s->encryption == 128 || s->encryption == 256))
+        peer_config->key_size = s->encryption;
+
+    if (s->buffer_size) {
+        peer_config->recovery_length_min = s->buffer_size;
+        peer_config->recovery_length_max = s->buffer_size;
+    }
+
+    ret = rist_peer_create(s->ctx, &s->peer, &s->peer_config);
+    if (ret < 0)
+        goto err;
+
+    ret = rist_start(s->ctx);
+    if (ret < 0)
+        goto err;
+
+    h->max_packet_size = 9968;
+
+    return 0;
+
+err:
+    librist_close(h);
+
+    return risterr2ret(ret);
+}
+
+static int librist_read(URLContext *h, uint8_t *buf, int size)
+{
+    RISTContext *s = h->priv_data;
+    const struct rist_data_block *data_block;
+    int ret;
+
+    ret = rist_receiver_data_read(s->ctx, &data_block, s->queue_count <= 0 ? POLLING_TIME : 0);
+    if (ret < 0)
+        return risterr2ret(ret);
+
+    if (ret == 0 || data_block->payload_len <= 0)
+        return 0;
+
+    s->queue_count = ret - 1;
+    size = data_block->payload_len;
+
+    memcpy(buf, data_block->payload, size);
+    rist_receiver_data_block_free((struct rist_data_block**)&data_block);
+
+    return size;
+}
+
+static int librist_write(URLContext *h, const uint8_t *buf, int size)
+{
+    RISTContext *s = h->priv_data;
+    struct rist_data_block data_block = { 0 };
+    int ret;
+
+    data_block.ts_ntp = 0;
+    data_block.payload = buf;
+    data_block.payload_len = size;
+
+    ret = rist_sender_data_write(s->ctx, &data_block);
+    if (ret < 0)
+        return risterr2ret(ret);
+
+    return ret;
+}
+
+static const AVClass librist_class = {
+    .class_name = "librist",
+    .item_name  = av_default_item_name,
+    .option     = librist_options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+const URLProtocol ff_librist_protocol = {
+    .name                = "rist",
+    .url_open            = librist_open,
+    .url_read            = librist_read,
+    .url_write           = librist_write,
+    .url_close           = librist_close,
+    .priv_data_size      = sizeof(RISTContext),
+    .flags               = URL_PROTOCOL_FLAG_NETWORK,
+    .priv_data_class     = &librist_class,
+};
diff --git a/libavformat/protocols.c b/libavformat/protocols.c
index 7df18fbb3b..c4fc446bb6 100644
--- a/libavformat/protocols.c
+++ b/libavformat/protocols.c
@@ -61,6 +61,7 @@  extern const URLProtocol ff_udp_protocol;
 extern const URLProtocol ff_udplite_protocol;
 extern const URLProtocol ff_unix_protocol;
 extern const URLProtocol ff_libamqp_protocol;
+extern const URLProtocol ff_librist_protocol;
 extern const URLProtocol ff_librtmp_protocol;
 extern const URLProtocol ff_librtmpe_protocol;
 extern const URLProtocol ff_librtmps_protocol;