On Wed, 23 Dec 2020, Paul B Mahol wrote:
On Wed, Dec 23, 2020 at 10:30 PM Marton Balint <c...@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
_______________________________________________
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".
