On Sat, 19 Dec 2020, Florian Levis wrote:

I use AMQP in PHP, .NET and also Java/Kotlin.
All clients have their way to handle the "options" ; usually they handle it
via the URI
It seems to be simplifier to handle all options related to a "connexion"
via an URI ; I see the same for e-mail & database connections strings (in
Symfony or Laravel for example).

So, should all the other already supported options (pkt_size, exchange,
routing_key, connection_timeout, delivery_mode) be passed in the URI?

Personally I don't really like this approach, but the main reason for that is that - unlike regular AVOptions which are parsed by the framework automatically - parsing them is a huge pain and extra code.

It might make sense to add support for the "official" query parameters:
https://www.rabbitmq.com/uri-query-parameters.html

But I'd rather not clutter existing code with adding URL option parsing for all the normal options we have.

Keep in mind that others may have other opinion about this, the ffmpeg codebase is not very consistent about which options can appear as URL parameters and which can not.

Regards,
Marton


--
Florian LEVIS


Le sam. 19 déc. 2020 à 18:41, Florian Levis <levis.flor...@gmail.com> a
écrit :

I have no idea how to do it.
I can take a look ; but I'm really not sure how to do it.

If one of you can handle it it might be safer ; if you can't, I will try.
Just let me know.

Thanks.

--
Florian LEVIS


Le sam. 19 déc. 2020 à 16:45, Andriy Gelman <andriy.gel...@gmail.com> a
écrit :

On Sat, 19. Dec 11:54, Marton Balint wrote:
>
>
> On Fri, 18 Dec 2020, Andriy Gelman wrote:
>
> > On Thu, 17. Dec 21:34, Florian Levis wrote:
> > > From: Florian Levis <fle...@hubee.tv>
> > >
> > > Add option vhost to allow publishing on other
> > > vhost than default '/'
> > >
> > > Signed-off-by: Florian Levis <levis.flor...@gmail.com>
> > > ---
> > >  doc/protocols.texi    | 3 +++
> > >  libavformat/libamqp.c | 4 +++-
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/doc/protocols.texi b/doc/protocols.texi
> > > index b4efa14509..8c7e4c7c52 100644
> > > --- a/doc/protocols.texi
> > > +++ b/doc/protocols.texi
> > > @@ -84,6 +84,9 @@ The following options are supported:
> > >
> > >  @table @option
> > >
> > > +@item vhost
> > > +Sets the vhost to use on the broker. Default is "/".
> > > +
> > >  @item exchange
> > >  Sets the exchange to use on the broker. RabbitMQ has several
predefined
> > >  exchanges: "amq.direct" is the default exchange, where the
publisher and
> > > diff --git a/libavformat/libamqp.c b/libavformat/libamqp.c
> > > index 81df724a6d..1465a4a133 100644
> > > --- a/libavformat/libamqp.c
> > > +++ b/libavformat/libamqp.c
> > > @@ -34,6 +34,7 @@ typedef struct AMQPContext {
> > >      const AVClass *class;
> > >      amqp_connection_state_t conn;
> > >      amqp_socket_t *socket;
> > > +    const char *vhost;
> > >      const char *exchange;
> > >      const char *routing_key;
> > >      int pkt_size;
> > > @@ -50,6 +51,7 @@ typedef struct AMQPContext {
> > >  #define E AV_OPT_FLAG_ENCODING_PARAM
> > >  static const AVOption options[] = {
> > >      { "pkt_size", "Maximum send/read packet size",
OFFSET(pkt_size), AV_OPT_TYPE_INT, { .i64 = 131072 }, 4096, INT_MAX, .flags
= D | E },
> >
> > > +    { "vhost", "vhost to send/read packets", OFFSET(vhost),
AV_OPT_TYPE_STRING, { .str = "/" }, 0, 0, .flags = D | E },
> >
> > I'll change the description to
> > "Name of virtual host on broker"
> >
> > >      { "exchange", "Exchange to send/read packets",
OFFSET(exchange), AV_OPT_TYPE_STRING, { .str = "amq.direct" }, 0, 0, .flags
= D | E },
> > >      { "routing_key", "Key to filter streams", OFFSET(routing_key),
AV_OPT_TYPE_STRING, { .str = "amqp" }, 0, 0, .flags = D | E },
> > >      { "connection_timeout", "Initial connection timeout",
OFFSET(connection_timeout), AV_OPT_TYPE_DURATION, { .i64 = -1 }, -1,
INT64_MAX, .flags = D | E},
> > > @@ -136,7 +138,7 @@ static int amqp_proto_open(URLContext *h, const
char *uri, int flags)
> > >          goto destroy_connection;
> > >      }
> > >
> > > -    broker_reply = amqp_login(s->conn, "/", 0, s->pkt_size, 0,
> > > +    broker_reply = amqp_login(s->conn, s->vhost, 0, s->pkt_size, 0,
> > >                                AMQP_SASL_METHOD_PLAIN,
user_decoded, password_decoded);
> > >
> > >      if (broker_reply.reply_type != AMQP_RESPONSE_NORMAL) {
> >
> > Will apply in the next few days.
>
> I think it is much better approach to parse the amqp URL to get the
vhost
> instead of adding a new option.
>
> https://www.rabbitmq.com/uri-spec.html
>
> amqp_URI       = "amqp://" amqp_authority [ "/" vhost ] [ "?" query ]

I agree, it does look a better approach.

--
Andriy


_______________________________________________
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".

Reply via email to