[FFmpeg-devel] [RFC][PATCH] libavformat: add librist protocol
Gijs Peskens
gijs at peskens.net
Mon Feb 22 10:51:38 EET 2021
Thanks for your swift reply! (and for your work on the patch, which I
believe I forgot to thank you for)
On 22-02-2021 09:17, Paul B Mahol wrote:
> On Mon, Feb 22, 2021 at 9:10 AM Gijs Peskens <gijs at peskens.net> wrote:
>
>> Hi, some feedback from one of the libRIST developers:
>>
>> Testing this patch (via diff found
>> on:
>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210126205145.20448-1-onemda@gmail.com/
>> ) I've noticed FFmpeg is not reading fast enough from the libRIST FIFO
>> buffer (which is limited to 1024 packets) causing it to overrun.
>> Unfortunately libRIST had a bug where it kept incrementing the buffer
>> counter indefinitely, this has now been fixed and it will log an error
>> whenever this happens (which is very often with FFmpeg).
>> FFmpeg will need to read the FIFO faster, by calling the librist_read
>> function more often. If that's not possible within the design of FFmpeg
>> the module can either:
>>
>
> Unfortunately some other devs disagree with initial patch and thus this
> issue.
I don't think this is as much a matter of opinion as it is a matter of
fact ;) Right now we (libRIST) have the output FIFO limited to 1024
packets, I don't see that changing in the near future. Right now FFmpeg
RIST input is simply broken, since the FIFO is overflowing repeatedly by
as much as 5-600 packets. When the FIFO is full libRIST will (by
design) free the oldest packet in it when adding a new packet to the
FIFO, so reading it at a lower rate than packet insertion will
inevitably lead to discontinuities, which RIST is supposed to prevent.
So if the FIFO is to be used it /must/ be read at a rate high enough to
prevent overflow.
>
>> -use the callback (runs within a libRIST thread context) and store the
>> data_block ptrs in a FIFO
>>
> That can not be used by FFmpeg API unfortunately.
>
>
>> -dedicate a thread and store the data_block ptrs in a FIFO
>> The librist_read function can then pop from the fifo, copy out the data,
>> and free the block.
>>
>> Also I suggest to set the default loglevel at least to RIST_LOG_ERROR
>> preferably at least to RIST_LOG_WARN.
>>
>> I'd also suggest changing the default buffer size and the min/max
>> values, the unit is ms. I'd suggest a min of 1 (0 would select the
>> default of libRIST -> 1000) a max somewhere far out enough but limited
>> enough to prevent shooting in the foot (i.e.: 30 seconds worth) and a
>> default of 1000.
>>
>> You can consider disabling advanced profile, libRIST limits profile to
>> MAIN atm, and advanced will likely not be out (VSF spec) before summer
>> (and quite likely much later).
>>
>> How does FFmpeg handle multiplexed streams from libRIST currently? This
>> is not clear to me. I'd think at the very least FFmpeg would need to
>> allow the user to filter to 1 stream based on the tunneled destination
>> port number, as the RIST protocol allows multiple streams to be tunneled
>> into 1 MAIN profile session.
>>
>> Personally I'd also like to be able to get the stats out of ffmpeg, an
>> option for that would be very welcome (simply writing out to a file
>> would be very helpful).
>>
>>
>> Sincerely,
>>
>>
>> Gijs Peskens
>>
>> On 22-12-2020 19:09, onemda at gmail.com (Paul B Mahol) wrote:
>>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>>> ---
>>> TODO:
>>> What about logging messages?
>>> Return codes?
>>> File handles?
>>> Add support for password?
>>> Advanced stuff?
>>>
>>> ---
>>> configure | 5 ++
>>> libavformat/Makefile | 1 +
>>> libavformat/librist.c | 164 ++++++++++++++++++++++++++++++++++++++++
>>> libavformat/protocols.c | 1 +
>>> 4 files changed, 171 insertions(+)
>>> create mode 100644 libavformat/librist.c
>>>
>>> diff --git a/configure b/configure
>>> index 90914752f1..462f2dcf64 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
>>> @@ -3488,6 +3490,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"
>>> @@ -6404,6 +6408,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.1.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/libavformat/Makefile b/libavformat/Makefile
>>> index 97d868081b..799e16c59e 100644
>>> --- a/libavformat/Makefile
>>> +++ b/libavformat/Makefile
>>> @@ -652,6 +652,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..b07f1ab673
>>> --- /dev/null
>>> +++ b/libavformat/librist.c
>>> @@ -0,0 +1,164 @@
>>> +/*
>>> + * 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 fd;
>>> +
>>> + int profile;
>>> +
>>> + const struct rist_peer_config *peer_config;
>>> + struct rist_peer *peer;
>>> + struct rist_ctx *rist_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[] = {
>>> + { "profile", "set RIST profile", OFFSET(profile), AV_OPT_TYPE_INT,
>> {.i64=RIST_PROFILE_SIMPLE}, 0, 2, .flags = D|E },
>>> + { NULL }
>>> +};
>>> +
>>> +static int librist_open(URLContext *h, const char *uri, int flags)
>>> +{
>>> + RISTContext *s = h->priv_data;
>>> + int ret;
>>> +
>>> + if (flags & AVIO_FLAG_WRITE) {
>>> + ret = rist_sender_create(&s->rist_ctx, s->profile, 0, NULL);
>>> + if (ret < 0)
>>> + return ret;
>>> + } else {
>>> + ret = rist_receiver_create(&s->rist_ctx, s->profile, NULL);
>>> + if (ret < 0)
>>> + return ret;
>>> + }
>>> +
>>> + ret = rist_parse_address(uri, &s->peer_config);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = rist_peer_create(s->rist_ctx, &s->peer, s->peer_config);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = rist_start(s->rist_ctx);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +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->rist_ctx, &data_block, 5);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (ret > 0) {
>>> + memcpy(buf, data_block->payload, data_block->payload_len);
>>> + return data_block->payload_len;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +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->rist_ctx, &data_block);
>>> + if (ret < 0)
>>> + return ret;
>>> + return size; // XXX should be ret actually but librist is buggy,
>> got OOM otherwise
>>> +}
>>> +
>>> +static int librist_close(URLContext *h)
>>> +{
>>> + RISTContext *s = h->priv_data;
>>> + int ret;
>>> +
>>> + free((void *)s->peer_config);
>>> + s->peer_config = NULL;
>>> +
>>> + ret = rist_peer_destroy(s->rist_ctx, s->peer);
>>> + if (ret < 0)
>>> + return ret;
>>> + s->peer = NULL;
>>> +
>>> + ret = rist_destroy(s->rist_ctx);
>>> + if (ret < 0)
>>> + return ret;
>>> + s->rist_ctx = NULL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int librist_get_file_handle(URLContext *h)
>>> +{
>>> + RISTContext *s = h->priv_data;
>>> +
>>> + return s->fd;
>>> +}
>>> +
>>> +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,
>>> + .url_get_file_handle = librist_get_file_handle,
>>> + .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;
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list