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

[James Almer]

On 12/23/2020 8:32 PM, Paul B Mahol wrote:
> +static int librist_close(URLContext *h)
> +{
> +    RISTContext *s = h->priv_data;
> +    int ret = 0;
> +
> +    s->peer = NULL;
> +
> +    if (s->rist_ctx)
> +        ret = rist_destroy(s->rist_ctx);
> +    if (ret < 0)
> +        return risterr2ret(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);

This is not a good idea. Afaik there could be more than one c library at 
play, the one linked by librist, and the one linked by libavformat.
This is why most libraries always end up adding custom free functions.

I see that rist_logging_set(), or anything in logging.h from this 
library for that matter, is undocumented, but reading the source code it 
seems it accepts your own allocated rist_logging_settings struct.
So IMO first consider asking for some doxy addition to the functions in 
order to make that officially supported, and then allocate it yourself 
in librist_open() with av_malloc(sizeof(*s->logging_settings)). 
Alternatively, they could provide a new rist_logging_free() function of 
sorts.

And same for peer_config above, except that rist_parse_address() does 
not accept your own allocated rist_peer_config struct. It only accepts 
an already initialized one from a previous call, so either that's 
changed, or a custom free() or a reset_to_defaults() function is added.

> +    s->logging_settings = NULL;
> +
> +    return 0;
> +}
> +
> +static int librist_open(URLContext *h, const char *uri, int flags)
> +{
> +    RISTContext *s = h->priv_data;
> +    int ret;
> +
> +    ret = rist_logging_set(&s->logging_settings, s->log_level, log_cb, h, NULL, stderr);
> +    if (ret < 0)
> +        return risterr2ret(ret);