[FFmpeg-devel] [PATCH] Make ff_url_split() and ff_url_join() public.

[Martin Storsjö]

On Sat, 22 May 2010, Stefano Sabatini wrote:

> ---
>  libavformat/avformat.h  |   45 ++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/internal.h  |   42 +++-------------------------------------
>  libavformat/rtmpproto.c |    4 +-
>  libavformat/utils.c     |   48 +++++++++++++++++++++++++++++++++++++++++-----
>  4 files changed, 93 insertions(+), 46 deletions(-)

Looks sensible to me in general. Please wait for a go or no-go from at 
least Ronald, too. Some comments below:

> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 358959c..142c4bc 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -89,27 +89,9 @@ int ff_probe_input_buffer(ByteIOContext **pb, AVInputFormat **fmt,
>                            const char *filename, void *logctx,
>                            unsigned int offset, unsigned int max_probe_size);
>  
> +#if LIBAVFORMAT_VERSION_MAJOR < 53
>  /**
> - * Splits a URL string into components. To reassemble components back into
> - * a URL, use ff_url_join instead of using snprintf directly.
> - *
> - * The pointers to buffers for storing individual components may be null,
> - * in order to ignore that component. Buffers for components not found are
> - * set to empty strings. If the port isn't found, it is set to a negative
> - * value.
> - *
> - * @see ff_url_join
> - *
> - * @param proto the buffer for the protocol
> - * @param proto_size the size of the proto buffer
> - * @param authorization the buffer for the authorization
> - * @param authorization_size the size of the authorization buffer
> - * @param hostname the buffer for the host name
> - * @param hostname_size the size of the hostname buffer
> - * @param port_ptr a pointer to store the port number in
> - * @param path the buffer for the path
> - * @param path_size the size of the path buffer
> - * @param url the URL to split
> + * @deprecated Use av_url_split() instead.
>   */
>  void ff_url_split(char *proto, int proto_size,
>                    char *authorization, int authorization_size,

Perhaps attribute_deprecated, too? And I guess these prototypes can be 
removed (or moved into the .c file, to avoid errors due to missing 
protoypes) after changing all places where it is used.


>  /**
> - * Assembles a URL string from components. This is the reverse operation
> - * of ff_url_split.
> - *
> - * Note, this requires networking to be initialized, so the caller must
> - * ensure ff_network_init has been called.
> - *
> - * @see ff_url_split
> - *
> - * @param str the buffer to fill with the url
> - * @param size the size of the str buffer
> - * @param proto the protocol identifier, if null, the separator
> - *              after the identifier is left out, too
> - * @param authorization an optional authorization string, may be null
> - * @param hostname the host name string
> - * @param port the port number, left out from the string if negative
> - * @param fmt a generic format string for everything to add after the
> - *            host/port, may be null
> - * @return the number of characters written to the destination buffer
> + * @deprecated Use av_url_join() instead.
>   */
>  int ff_url_join(char *str, int size, const char *proto,
>                  const char *authorization, const char *hostname,
>                  int port, const char *fmt, ...);
> +#endif

Dito. After changing all usage of ff_url_join, you shouldn't need to keep 
the function at all, it was never used outside of lavf as far as I know.

> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
> index 4edbffa..de63d79 100644
> --- a/libavformat/rtmpproto.c
> +++ b/libavformat/rtmpproto.c
> @@ -114,7 +114,7 @@ static void gen_connect(URLContext *s, RTMPContext *rt, const char *proto,
>      ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0, 4096);
>      p = pkt.data;
>  
> -    ff_url_join(tcurl, sizeof(tcurl), proto, NULL, host, port, "/%s", rt->app);
> +    ff_join_url(tcurl, sizeof(tcurl), proto, NULL, host, port, "/%s", rt->app);
>      ff_amf_write_string(&p, "connect");
>      ff_amf_write_number(&p, 1.0);

Accidentally added?

> -int ff_url_join(char *str, int size, const char *proto,
> -                const char *authorization, const char *hostname,
> -                int port, const char *fmt, ...)
> +static int av_url_vjoin(char *str, int size, const char *proto,
> +                        const char *authorization, const char *hostname,
> +                        int port, const char *fmt, va_list vl)

Hmm, I'm not sure if the function should be named av_ if it's static, 
should it?

// Martin