| Message ID | 20201223181029.17021-1-onemda@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] libavformat: add librist protocol | expand |
| 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 |
On Wed, 23 Dec 2020, 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 | 227 ++++++++++++++++++++++++++++++++++++++++ > libavformat/protocols.c | 1 + > 5 files changed, 263 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/doc/protocols.texi b/doc/protocols.texi > index de377a9546..58627040f5 100644 > --- a/doc/protocols.texi > +++ b/doc/protocols.texi > @@ -673,6 +673,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 receiving and sending 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. What about 192? > +@end table Missing docs for pkt_size. Some usage examples could be useful. > + > @section rtmp > > Real-Time Messaging Protocol. > 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..dbffb5cdc1 > --- /dev/null > +++ b/libavformat/librist.c > @@ -0,0 +1,227 @@ > +/* > + * 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; uint32_t. Also you should probably name this flow_id, not fd, as it is not a file descriptor. > + > + int profile; > + int buffer_size; > + int packet_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 *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[] = { > + { "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 }, > + { "pkt_size", "set packet size", OFFSET(packet_size), AV_OPT_TYPE_INT, {.i64=INT_MAX}, 1, INT_MAX, .flags = E }, INT_MAX for pkt_size is probably not a good default. pkt_size is used (at least in other protocols) to determine the maximum allowed packet size for output, and the maximum supported packet size for input. I am not sure what you can pump into RIST (mpegts most likely?) but probably it should be determined for that usage. e.g. 1316? Also the URLContext->max_packet_size should be set based on the packet_size. > + { "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=""}, 0, 0, .flags = D|E }, default should rather be NULL, no? > + { "encryption","set encryption type",OFFSET(encryption), AV_OPT_TYPE_INT ,{.i64=0}, 0, INT_MAX, .flags = D|E }, > + { NULL } > +}; > + > +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_open(URLContext *h, const char *uri, int flags) > +{ > + RISTContext *s = h->priv_data; > + int ret; > + > + s->fd = rist_flow_id_create(); > + > + ret = rist_logging_set(&s->logging_settings, s->log_level, log_cb, h, NULL, stderr); stderr should not be given, because we are using our own log callback. > + if (ret < 0) > + return ret; RIST errors should be mapped to ffmpeg errors. It seems rist is using -1 everywhere, so that should be mapped to AVERROR_EXTERNAL if there is no way to acquire more precise error. You are leaking logging_settings, and a lot of other stuff on failure paths after this. url_close is only called automatically if url_open was successful. > + > + if (flags & AVIO_FLAG_WRITE) { > + ret = rist_sender_create(&s->rist_ctx, s->profile, s->fd, s->logging_settings); > + if (ret < 0) > + return ret; > + } else { > + ret = rist_receiver_create(&s->rist_ctx, s->profile, s->logging_settings); > + if (ret < 0) > + return ret; > + } > + > + ret = rist_parse_address(uri, (void *)&s->peer_config); > + if (ret < 0) > + return ret; > + > + if (((s->encryption == 128 || s->encryption == 256) && !s->secret) || > + ((s->peer_config->key_size == 128 || s->peer_config->key_size == 256) && !s->peer_config->secret[0])) { > + av_log(h, AV_LOG_ERROR, "secret is mandatory if encryption is enabled\n"); > + return AVERROR(EINVAL); > + } 192-byte encryption is also supported as far as I see. > + > + if (s->secret && s->peer_config->secret[0] == 0) > + strncpy(s->peer_config->secret, s->secret, FFMIN(RIST_MAX_STRING_SHORT - 1, strlen(s->secret))); av_strlcpy > + > + if (s->secret && (s->encryption == 128 || s->encryption == 256)) > + s->peer_config->key_size = s->encryption; Some error handling should be done, e.g. reject encryption sizes not supported, reject encryption without secret given, etc. > + > + if (s->buffer_size) { > + s->peer_config->recovery_length_min = s->buffer_size; > + s->peer_config->recovery_length_max = s->buffer_size; > + } > + > + 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) { > + size = FFMIN(size, data_block->payload_len); > + memcpy(buf, data_block->payload, size); Some error message about the truncation should be logged, or maybe a hard error is even better if a truncation would occur? > + return size; > + } > + > + return 0; This should return AVERROR(EAGAIN). > +} > + > +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 = FFMIN(s->packet_size, size); If you set h->max_packet_size then this is ensured by the framework. > + ret = rist_sender_data_write(s->rist_ctx, &data_block); > + if (ret < 0) > + return ret; > + else if (ret) > + return ret; > + else // remove this part when new librist release appears > + return size; > +} > + > +static int librist_close(URLContext *h) > +{ > + RISTContext *s = h->priv_data; > + int ret; > + > + s->peer = NULL; > + > + ret = rist_destroy(s->rist_ctx); > + if (ret < 0) > + return ret; > + s->rist_ctx = NULL; > + > + if (s->peer_config) > + free((void *)s->peer_config); > + s->peer_config = NULL; > + > + if (s->logging_settings) > + free((void *)s->logging_settings); > + s->logging_settings = NULL; > + > + return 0; > +} > + > +static int librist_get_file_handle(URLContext *h) > +{ > + RISTContext *s = h->priv_data; > + > + return s->fd; > +} I don't think this is right, s->fd is a flow id, not an ordinary file descriptor. You probably don't need this callback anyway. > + > +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; Regards, Marton
On Wed, Dec 23, 2020 at 10:30 PM Marton Balint <cus@passwd.hu> wrote: > > > On Wed, 23 Dec 2020, 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 | 227 ++++++++++++++++++++++++++++++++++++++++ > > libavformat/protocols.c | 1 + > > 5 files changed, 263 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/doc/protocols.texi b/doc/protocols.texi > > index de377a9546..58627040f5 100644 > > --- a/doc/protocols.texi > > +++ b/doc/protocols.texi > > @@ -673,6 +673,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 receiving and sending 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. > > What about 192? > Unofficial. > > > +@end table > > Missing docs for pkt_size. > > Some usage examples could be useful. > > > + > > @section rtmp > > > > Real-Time Messaging Protocol. > > 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..dbffb5cdc1 > > --- /dev/null > > +++ b/libavformat/librist.c > > @@ -0,0 +1,227 @@ > > +/* > > + * 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; > > uint32_t. Also you should probably name this flow_id, not fd, as it is not > a file descriptor. > But it is used for same thing. > > > + > > + int profile; > > + int buffer_size; > > + int packet_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 *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[] = { > > + { "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 }, > > + { "pkt_size", "set packet size", OFFSET(packet_size), > AV_OPT_TYPE_INT, {.i64=INT_MAX}, 1, INT_MAX, .flags = E }, > > INT_MAX for pkt_size is probably not a good default. pkt_size is used (at > least in other protocols) to determine the maximum allowed packet size for > output, and the maximum supported packet size for input. I am not sure > what you can pump into RIST (mpegts most likely?) but probably it should > be determined for that usage. e.g. 1316? > > Also the URLContext->max_packet_size should be set based on the > packet_size. > > > + { "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=""}, 0, 0, .flags = D|E }, > > default should rather be NULL, no? > Same thing. > > > + { "encryption","set encryption type",OFFSET(encryption), > AV_OPT_TYPE_INT ,{.i64=0}, 0, INT_MAX, .flags = D|E }, > > + { NULL } > > +}; > > + > > +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_open(URLContext *h, const char *uri, int flags) > > +{ > > + RISTContext *s = h->priv_data; > > + int ret; > > + > > + s->fd = rist_flow_id_create(); > > + > > + ret = rist_logging_set(&s->logging_settings, s->log_level, log_cb, > h, NULL, stderr); > > stderr should not be given, because we are using our own log callback. > Not used, thus not really relevant. > > > + if (ret < 0) > > + return ret; > > RIST errors should be mapped to ffmpeg errors. It seems rist is using -1 > everywhere, so that should be mapped to AVERROR_EXTERNAL if there is no > way to acquire more precise error. > > You are leaking logging_settings, and a lot of other stuff on failure > paths after this. url_close is only called automatically if url_open was > successful. > > > > + > > + if (flags & AVIO_FLAG_WRITE) { > > + ret = rist_sender_create(&s->rist_ctx, s->profile, s->fd, > s->logging_settings); > > + if (ret < 0) > > + return ret; > > + } else { > > + ret = rist_receiver_create(&s->rist_ctx, s->profile, > s->logging_settings); > > + if (ret < 0) > > + return ret; > > + } > > + > > + ret = rist_parse_address(uri, (void *)&s->peer_config); > > + if (ret < 0) > > + return ret; > > + > > + if (((s->encryption == 128 || s->encryption == 256) && !s->secret) > || > > + ((s->peer_config->key_size == 128 || s->peer_config->key_size > == 256) && !s->peer_config->secret[0])) { > > + av_log(h, AV_LOG_ERROR, "secret is mandatory if encryption is > enabled\n"); > > + return AVERROR(EINVAL); > > + } > > 192-byte encryption is also supported as far as I see. > Not officially. > > > + > > + if (s->secret && s->peer_config->secret[0] == 0) > > + strncpy(s->peer_config->secret, s->secret, > FFMIN(RIST_MAX_STRING_SHORT - 1, strlen(s->secret))); > > av_strlcpy > > > + > > + if (s->secret && (s->encryption == 128 || s->encryption == 256)) > > + s->peer_config->key_size = s->encryption; > > Some error handling should be done, e.g. reject encryption sizes not > supported, reject encryption without secret given, etc. > That is already handled. > > > + > > + if (s->buffer_size) { > > + s->peer_config->recovery_length_min = s->buffer_size; > > + s->peer_config->recovery_length_max = s->buffer_size; > > + } > > + > > + 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) { > > + size = FFMIN(size, data_block->payload_len); > > + memcpy(buf, data_block->payload, size); > > Some error message about the truncation should be logged, or maybe a hard > error is even better if a truncation would occur? > > > > + return size; > > + } > > + > > + return 0; > > This should return AVERROR(EAGAIN). > > > +} > > + > > +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 = FFMIN(s->packet_size, size); > > If you set h->max_packet_size then this is ensured by the framework. > > > + ret = rist_sender_data_write(s->rist_ctx, &data_block); > > + if (ret < 0) > > + return ret; > > + else if (ret) > > + return ret; > > + else // remove this part when new librist release appears > > + return size; > > +} > > + > > +static int librist_close(URLContext *h) > > +{ > > + RISTContext *s = h->priv_data; > > + int ret; > > + > > + s->peer = NULL; > > + > > + ret = rist_destroy(s->rist_ctx); > > + if (ret < 0) > > + return ret; > > + s->rist_ctx = NULL; > > + > > + if (s->peer_config) > > + free((void *)s->peer_config); > > + s->peer_config = NULL; > > + > > + if (s->logging_settings) > > + free((void *)s->logging_settings); > > + s->logging_settings = NULL; > > + > > + return 0; > > +} > > + > > +static int librist_get_file_handle(URLContext *h) > > +{ > > + RISTContext *s = h->priv_data; > > + > > + return s->fd; > > +} > > I don't think this is right, s->fd is a flow id, not an ordinary file > descriptor. You probably don't need this callback anyway. > Are you really sure it is not needed? > > + > > +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; > > 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 (12020-12-23): > > +static int librist_get_file_handle(URLContext *h) > > +{ > > + RISTContext *s = h->priv_data; > > + > > + return s->fd; > > +} > > I don't think this is right, s->fd is a flow id, not an ordinary file > descriptor. You probably don't need this callback anyway. It is indeed not a file descriptor at all, returning it like that would cause very serious bugs. It seems this library does not make its file descriptors at all. It means it cannot be used by FFmpeg in non-blocking mode, nor by any application with a standard Unix event loop. Regards,
On Wed, Dec 23, 2020 at 11:29 PM Nicolas George <george@nsup.org> wrote: > Marton Balint (12020-12-23): > > > +static int librist_get_file_handle(URLContext *h) > > > +{ > > > + RISTContext *s = h->priv_data; > > > + > > > + return s->fd; > > > +} > > > > I don't think this is right, s->fd is a flow id, not an ordinary file > > descriptor. You probably don't need this callback anyway. > > It is indeed not a file descriptor at all, returning it like that would > cause very serious bugs. > > I nowhere see proof that this cause any bugs. > It seems this library does not make its file descriptors at all. It > means it cannot be used by FFmpeg in non-blocking mode, nor by any > application with a standard Unix event loop This is fortunately completely false statement. It works just fine. . > > Regards, > > -- > Nicolas George > _______________________________________________ > 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".
Paul B Mahol (12020-12-23): > This work is sponsored by Open Broadcast Systems. Thanks for the disclosure. > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > configure | 5 + > doc/protocols.texi | 29 +++++ > libavformat/Makefile | 1 + > libavformat/librist.c | 227 ++++++++++++++++++++++++++++++++++++++++ > libavformat/protocols.c | 1 + > 5 files changed, 263 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/doc/protocols.texi b/doc/protocols.texi > index de377a9546..58627040f5 100644 > --- a/doc/protocols.texi > +++ b/doc/protocols.texi > @@ -673,6 +673,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 receiving and sending 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 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..dbffb5cdc1 > --- /dev/null > +++ b/libavformat/librist.c > @@ -0,0 +1,227 @@ > +/* > + * 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; > + int buffer_size; > + int packet_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 *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[] = { > + { "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 }, > + { "pkt_size", "set packet size", OFFSET(packet_size), AV_OPT_TYPE_INT, {.i64=INT_MAX}, 1, INT_MAX, .flags = 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=""}, 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 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_open(URLContext *h, const char *uri, int flags) > +{ > + RISTContext *s = h->priv_data; > + int ret; > + > + s->fd = rist_flow_id_create(); > + > + ret = rist_logging_set(&s->logging_settings, s->log_level, log_cb, h, NULL, stderr); > + if (ret < 0) > + return ret; Here and everywhere: the return values of this library have nothing to do with AVERROR codes, they cannot be returned like this. > + > + if (flags & AVIO_FLAG_WRITE) { > + ret = rist_sender_create(&s->rist_ctx, s->profile, s->fd, s->logging_settings); > + if (ret < 0) > + return ret; > + } else { > + ret = rist_receiver_create(&s->rist_ctx, s->profile, s->logging_settings); > + if (ret < 0) > + return ret; > + } It should return an error for AVIO_FLAG_READ_WRITE. > + > + ret = rist_parse_address(uri, (void *)&s->peer_config); This cast looks wrong. s->peer_config should probably be const. Same for others. > + if (ret < 0) > + return ret; > + > + if (((s->encryption == 128 || s->encryption == 256) && !s->secret) || > + ((s->peer_config->key_size == 128 || s->peer_config->key_size == 256) && !s->peer_config->secret[0])) { > + av_log(h, AV_LOG_ERROR, "secret is mandatory if encryption is enabled\n"); > + return AVERROR(EINVAL); > + } > + > + if (s->secret && s->peer_config->secret[0] == 0) > + strncpy(s->peer_config->secret, s->secret, FFMIN(RIST_MAX_STRING_SHORT - 1, strlen(s->secret))); > + > + if (s->secret && (s->encryption == 128 || s->encryption == 256)) > + s->peer_config->key_size = s->encryption; > + > + if (s->buffer_size) { > + s->peer_config->recovery_length_min = s->buffer_size; > + s->peer_config->recovery_length_max = s->buffer_size; > + } > + > + 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); The timeout should be much longer. > + if (ret < 0) > + return ret; > + > + if (ret > 0) { > + size = FFMIN(size, data_block->payload_len); > + memcpy(buf, data_block->payload, size); > + return size; > + } > + > + return 0; This is reached when no packet is available in the timeout, but returning 0 here means a 0-sized packet. This is not correct. > +} > + > +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 = FFMIN(s->packet_size, size); > + ret = rist_sender_data_write(s->rist_ctx, &data_block); > + if (ret < 0) > + return ret; > + else if (ret) > + return ret; > + else // remove this part when new librist release appears > + return size; > +} > + > +static int librist_close(URLContext *h) > +{ > + RISTContext *s = h->priv_data; > + int ret; > + > + s->peer = NULL; > + > + ret = rist_destroy(s->rist_ctx); > + if (ret < 0) > + return ret; > + s->rist_ctx = NULL; > + > + if (s->peer_config) > + free((void *)s->peer_config); > + s->peer_config = NULL; > + > + if (s->logging_settings) > + free((void *)s->logging_settings); > + s->logging_settings = NULL; > + > + return 0; > +} > + > +static int librist_get_file_handle(URLContext *h) > +{ > + RISTContext *s = h->priv_data; > + > + return s->fd; > +} As pointed, this is completely wrong. And therefore, fd should be completely local. > + > +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; Regards,
Paul B Mahol (12020-12-23): > I nowhere see proof that this cause any bugs. get_file_handle() returns a file descriptor for use with select() or poll(). If you give it a random number (fd is a random number), the application will poll it as if it were a file descriptor. At best, it causes EBADFD. At worse the application starts polling an existing and unrelated file descriptor, causing random behaviors. > This is fortunately completely false statement. It works just fine. It works fine because this part of the code is completely not tested by any of the fftools.
On Wed, 23 Dec 2020, Paul B Mahol wrote: > On Wed, Dec 23, 2020 at 10:30 PM Marton Balint <cus@passwd.hu> wrote: > >> >> >> >> INT_MAX for pkt_size is probably not a good default. pkt_size is used (at >> least in other protocols) to determine the maximum allowed packet size for >> output, and the maximum supported packet size for input. I am not sure >> what you can pump into RIST (mpegts most likely?) but probably it should >> be determined for that usage. e.g. 1316? >> >> Also the URLContext->max_packet_size should be set based on the >> packet_size. >> >> > + { "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=""}, 0, 0, .flags = D|E }, >> >> default should rather be NULL, no? >> > > Same thing. Surely there is no significant difference, I just feel that it cleaner to use NULL as unspecified default. [...] >> > +static int librist_open(URLContext *h, const char *uri, int flags) >> > +{ >> > + RISTContext *s = h->priv_data; >> > + int ret; >> > + >> > + s->fd = rist_flow_id_create(); >> > + >> > + ret = rist_logging_set(&s->logging_settings, s->log_level, log_cb, >> h, NULL, stderr); >> >> stderr should not be given, because we are using our own log callback. >> > > Not used, thus not really relevant. OK, but it still confused me that stderr may be used for anything. So yet again I believe it is cleaner to use NULL. [...] >> Some error handling should be done, e.g. reject encryption sizes not >> supported, reject encryption without secret given, etc. >> > > That is already handled. As far as I see invalid encryption sizes are not rejected but silently ignored. >> > +static int librist_get_file_handle(URLContext *h) >> > +{ >> > + RISTContext *s = h->priv_data; >> > + >> > + return s->fd; >> > +} >> >> I don't think this is right, s->fd is a flow id, not an ordinary file >> descriptor. You probably don't need this callback anyway. >> > > Are you really sure it is not needed? Nicolas already explained the use case for this callback, so yes, I think it is not needed. Regards, Marton
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/doc/protocols.texi b/doc/protocols.texi index de377a9546..58627040f5 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -673,6 +673,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 receiving and sending 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 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..dbffb5cdc1 --- /dev/null +++ b/libavformat/librist.c @@ -0,0 +1,227 @@ +/* + * 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; + int buffer_size; + int packet_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 *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[] = { + { "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 }, + { "pkt_size", "set packet size", OFFSET(packet_size), AV_OPT_TYPE_INT, {.i64=INT_MAX}, 1, INT_MAX, .flags = 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=""}, 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 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_open(URLContext *h, const char *uri, int flags) +{ + RISTContext *s = h->priv_data; + int ret; + + s->fd = rist_flow_id_create(); + + ret = rist_logging_set(&s->logging_settings, s->log_level, log_cb, h, NULL, stderr); + if (ret < 0) + return ret; + + if (flags & AVIO_FLAG_WRITE) { + ret = rist_sender_create(&s->rist_ctx, s->profile, s->fd, s->logging_settings); + if (ret < 0) + return ret; + } else { + ret = rist_receiver_create(&s->rist_ctx, s->profile, s->logging_settings); + if (ret < 0) + return ret; + } + + ret = rist_parse_address(uri, (void *)&s->peer_config); + if (ret < 0) + return ret; + + if (((s->encryption == 128 || s->encryption == 256) && !s->secret) || + ((s->peer_config->key_size == 128 || s->peer_config->key_size == 256) && !s->peer_config->secret[0])) { + av_log(h, AV_LOG_ERROR, "secret is mandatory if encryption is enabled\n"); + return AVERROR(EINVAL); + } + + if (s->secret && s->peer_config->secret[0] == 0) + strncpy(s->peer_config->secret, s->secret, FFMIN(RIST_MAX_STRING_SHORT - 1, strlen(s->secret))); + + if (s->secret && (s->encryption == 128 || s->encryption == 256)) + s->peer_config->key_size = s->encryption; + + if (s->buffer_size) { + s->peer_config->recovery_length_min = s->buffer_size; + s->peer_config->recovery_length_max = s->buffer_size; + } + + 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) { + size = FFMIN(size, data_block->payload_len); + memcpy(buf, data_block->payload, size); + return size; + } + + 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 = FFMIN(s->packet_size, size); + ret = rist_sender_data_write(s->rist_ctx, &data_block); + if (ret < 0) + return ret; + else if (ret) + return ret; + else // remove this part when new librist release appears + return size; +} + +static int librist_close(URLContext *h) +{ + RISTContext *s = h->priv_data; + int ret; + + s->peer = NULL; + + ret = rist_destroy(s->rist_ctx); + if (ret < 0) + return ret; + s->rist_ctx = NULL; + + if (s->peer_config) + free((void *)s->peer_config); + s->peer_config = NULL; + + if (s->logging_settings) + free((void *)s->logging_settings); + s->logging_settings = 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;
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 | 227 ++++++++++++++++++++++++++++++++++++++++ libavformat/protocols.c | 1 + 5 files changed, 263 insertions(+) create mode 100644 libavformat/librist.c