[FFmpeg-devel] [PATCH] libavformat: add librist protocol

[Marton Balint]

On Fri, 26 Feb 2021, Paul B Mahol wrote:

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

> +#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 },

Why have you removed pkt_size? For writing, it is surely useful to be 
configurable, please put it back with -1 as default, so different default 
can be used for read and write. And for writing, the default of 1316 
should be used, as we typically want to pump mpegts into it, split on 
packet boundaries and want no fragmentation.

> +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_WRITE | AVIO_FLAG_READ)) == (AVIO_FLAG_WRITE | AVIO_FLAG_READ))
> +        return AVERROR(EINVAL);

AVIO_FLAG_READ_WRITE

> +
> +    return risterr2ret(ret);
> +}
> +
> +static int librist_read(URLContext *h, uint8_t *buf, int size)
> +{
> +    RISTContext *s = h->priv_data;
> +    int available_size = size;
> +    int queue_size = 0;
> +    int offset = 0;
> +
> +    do {
> +        const struct rist_data_block *data_block;
> +        int ret;
> +
> +        ret = rist_receiver_data_read(s->ctx, &data_block, offset == 0 ? POLLING_TIME : 0);
> +        if (ret < 0)
> +            return risterr2ret(ret);
> +
> +        if (ret == 0 || data_block->payload_len <= 0)
> +            break;
> +
> +        queue_size = ret;
> +        av_assert0(data_block->payload_len <= available_size);
> +
> +        size = FFMIN(available_size, data_block->payload_len);
> +        memcpy(buf + offset, data_block->payload, size);
> +        offset += size;
> +        available_size -= size;
> +        rist_receiver_data_block_free((struct rist_data_block**)&data_block);
> +        queue_size--;

No, read one packet in one read call, do not merge packets. Protocols 
don't merge packets. UDP does not merge packets. If this is some 
workaround for librist dropping packets then figure out the bug causing 
that. If UDP works, librist should work too, even without merging packets.

Regards,
Marton