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

[Marton Balint]

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.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
>>>
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".