On Mon, Feb 22, 2021 at 9:10 AM Gijs Peskens <g...@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-one...@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. > -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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
