[FFmpeg-devel] [PATCH] avformat/libamqp: add option vhost

[Florian Levis]

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?

--
Florian LEVIS


Le sam. 19 déc. 2020 à 18:41, Florian Levis <levis.florian at 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.gelman at 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 <flevis at hubee.tv>
>> > > >
>> > > > Add option vhost to allow publishing on other
>> > > > vhost than default '/'
>> > > >
>> > > > Signed-off-by: Florian Levis <levis.florian at 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
>> > > >
>> > > > + at 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
>>
>