[FFmpeg-devel] [PATCH 2/8] lavf/avio: add ffurl_accept and ffurl_handshake

[Nicolas George]

Le tridi 3 thermidor, an CCXXIII, Stephan Holljes a écrit :
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/avio.c | 19 +++++++++++++++++++
>  libavformat/url.h  | 18 ++++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/libavformat/avio.c b/libavformat/avio.c
> index c188adc..1182336 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
> @@ -211,6 +211,25 @@ int ffurl_connect(URLContext *uc, AVDictionary **options)
>      return 0;
>  }
>  
> +int ffurl_accept(URLContext *s, URLContext **c)
> +{

Maybe "av_assert0(!*c);" here would make sense: the application would have
to clear the pointer initially. That way, we are free to expand the API
later to allow using a context that was already allocated or something. If
you think it makes sense, do not forget to update the doxys.

> +    if (s->prot->url_accept)
> +        return s->prot->url_accept(s, c);

> +    return 0;

I had not spotted it earlier: an error code here would probably make more
sense. AVERROR(EBADF) would probably be the best choice, it is supposed to
be portable.

> +}
> +
> +int ffurl_handshake(URLContext *c)
> +{
> +    int ret;
> +    if (c->prot->url_handshake) {
> +        ret = c->prot->url_handshake(c);
> +        if (ret)
> +            return ret;
> +    }
> +    c->is_connected = 1;
> +    return 0;
> +}
> +
>  #define URL_SCHEME_CHARS                        \
>      "abcdefghijklmnopqrstuvwxyz"                \
>      "ABCDEFGHIJKLMNOPQRSTUVWXYZ"                \
> diff --git a/libavformat/url.h b/libavformat/url.h
> index 99a3201..d010a77 100644
> --- a/libavformat/url.h
> +++ b/libavformat/url.h
> @@ -58,6 +58,8 @@ typedef struct URLProtocol {
>       * for those nested protocols.
>       */
>      int     (*url_open2)(URLContext *h, const char *url, int flags, AVDictionary **options);
> +    int     (*url_accept)(URLContext *s, URLContext **c);

> +    int     (*url_handshake)(URLContext *c);

Since the API has become non-trivial, it needs a doxy. I suggest something
like this:

/**
 * Perform one step of the protocol handshake to accept a new client.
 * See avio_handshake() for details.
 * Implementations should try to return decreasing values.
 * If the protocol uses an underlying protocol, the underlying handshake is
 * usually the first step, and the return value can be:
 * (largest value for this protocol) + (return value from other protocol)
 */

It references a doxy that is in a later patch; this would be a deal breaker
if it was a function call, but for a doxy that seems acceptable.

The doxy for avio_handshake() must explain in more details:

/**
 * Perform one step of the protocol handshake to accept a new client.
 * This function must be called on a client returned by avio_accept() before
 * using as a read/write context.
 * It is separate from avio_accept() because it may block.
 * A step of the handshake is defined by places where the application may
 * decide to change the proceedings.
 * For example, on a protocol with a request header and a reply header, each
 * one can constitute a step because the application may use the parameters
 * from the request to change parameters in the reply; or each individual
 * chunk of the request can constitute a step.
 *
 * @return  0 if the handshake is complete and successful;
 *         >0 if the handshake has progressed but is not complete;
 *         <0 for an AVERROR code
 */

Feel free to alter the wording if you find better.

>  
>      /**
>       * Read data from the protocol.
> @@ -140,6 +142,22 @@ int ffurl_open(URLContext **puc, const char *filename, int flags,
>                 const AVIOInterruptCB *int_cb, AVDictionary **options);
>  
>  /**
> + * Accept an URLContext c on an URLContext s
> + * @param  s server context
> + * @param  c client context
> + * @return >= 0 on success, ff_neterrno() on failure.
> + */
> +int ffurl_accept(URLContext *s, URLContext **c);
> +
> +/**
> + * Perform a protocol handshake on the passed client context.
> + * @param  c the client context
> + * @return >= 0 on success or a negative value corresponding
> + *         to an AVERROR code on failure
> + */
> +int ffurl_handshake(URLContext *c);
> +
> +/**
>   * Read up to size bytes from the resource accessed by h, and store
>   * the read bytes in buf.
>   *

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150721/f91e8282/attachment.sig>