On Thu, 21 May 2015 22:07:39 +0300 (EEST)
Martin Storsjö <[email protected]> wrote:

> On Thu, 21 May 2015, Martin Storsjö wrote:
> 
> > On Thu, 21 May 2015, wm4 wrote:
> >
> >> On Thu, 21 May 2015 16:08:59 +0300
> >> Martin Storsjö <[email protected]> wrote:
> >>
> >>> This avoids hijacking the fd, by reading using the normal
> >>> URLContext functions instead. This allowing reading data that has
> >>> been buffered in the underlying URLContext.
> >>> 
> >>> This avoids using the libraries own send functions that can
> >>> cause SIGPIPE.
> >>> 
> >>> The fd is still needed for polling the lowlevel socket, for
> >>> waiting for retries.
> >>> ---
> >>> I realized I had an old patchset doing this lying around locally;
> >>> it still seems to work after rebasing, but some thorough testing
> >>> is welcome.
> >>> ---
> >>>  libavformat/tls.c | 105 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >>>  1 file changed, 103 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/libavformat/tls.c b/libavformat/tls.c
> >>> index d5de5ee..7a08e1d 100644
> >>> --- a/libavformat/tls.c
> >>> +++ b/libavformat/tls.c
> >>> @@ -36,6 +36,33 @@
> >>>          if (c->cred) \
> >>>              gnutls_certificate_free_credentials(c->cred); \
> >>>      } while (0)
> >>> +
> >>> +static ssize_t gnutls_url_pull(gnutls_transport_ptr_t transport,
> >>> +                               void *buf, size_t len)
> >>> +{
> >>> +    URLContext *h = (URLContext*) transport;
> >>> +    int ret = ffurl_read(h, buf, len);
> >>> +    if (ret >= 0)
> >>> +        return ret;
> >>> +    if (ret == AVERROR(EAGAIN))
> >>> +        errno = EAGAIN;
> >>> +    else
> >>> +        errno = EIO;
> >>> +    return -1;
> >>> +}
> >>> +static ssize_t gnutls_url_push(gnutls_transport_ptr_t transport,
> >>> +                               const void *buf, size_t len)
> >>> +{
> >>> +    URLContext *h = (URLContext*) transport;
> >>> +    int ret = ffurl_write(h, buf, len);
> >>> +    if (ret >= 0)
> >>> +        return ret;
> >>> +    if (ret == AVERROR(EAGAIN))
> >>> +        errno = EAGAIN;
> >>> +    else
> >>> +        errno = EIO;
> >>> +    return -1;
> >>> +}
> >>>  #elif CONFIG_OPENSSL
> >>>  #include <openssl/bio.h>
> >>>  #include <openssl/ssl.h>
> >>> @@ -49,6 +76,70 @@
> >>>          if (c->ctx) \
> >>>              SSL_CTX_free(c->ctx); \
> >>>      } while (0)
> >>> +
> >>> +static int url_bio_create(BIO *b)
> >>> +{
> >>> +    b->init = 1;
> >>> +    b->ptr = NULL;
> >>> +    b->flags = 0;
> >>> +    return 1;
> >>> +}
> >>> +
> >>> +static int url_bio_destroy(BIO *b)
> >>> +{
> >>> +    return 1;
> >>> +}
> >>> +
> >>> +static int url_bio_bread(BIO *b, char *buf, int len)
> >>> +{
> >>> +    URLContext *h = b->ptr;
> >>> +    int ret = ffurl_read(h, buf, len);
> >>> +    if (ret >= 0)
> >>> +        return ret;
> >>> +    BIO_clear_retry_flags(b);
> >>> +    if (ret == AVERROR(EAGAIN))
> >>> +        BIO_set_retry_read(b);
> >>> +    return -1;
> >>> +}
> >>> +
> >>> +static int url_bio_bwrite(BIO *b, const char *buf, int len)
> >>> +{
> >>> +    URLContext *h = b->ptr;
> >>> +    int ret = ffurl_write(h, buf, len);
> >>> +    if (ret >= 0)
> >>> +        return ret;
> >>> +    BIO_clear_retry_flags(b);
> >>> +    if (ret == AVERROR(EAGAIN))
> >>> +        BIO_set_retry_write(b);
> >>> +    return -1;
> >>> +}
> >>> +
> >>> +static long url_bio_ctrl(BIO *b, int cmd, long num, void *ptr)
> >>> +{
> >>> +    if (cmd == BIO_CTRL_FLUSH) {
> >>> +        BIO_clear_retry_flags(b);
> >>> +        return 1;
> >>> +    }
> >>> +    return 0;
> >>> +}
> >>> +
> >>> +static int url_bio_bputs(BIO *b, const char *str)
> >>> +{
> >>> +    return url_bio_bwrite(b, str, strlen(str));
> >>> +}
> >>> +
> >>> +static BIO_METHOD url_bio_method = {
> >>> +    .type = BIO_TYPE_SOURCE_SINK,
> >>> +    .name = "urlprotocol bio",
> >>> +    .bwrite = url_bio_bwrite,
> >>> +    .bread = url_bio_bread,
> >>> +    .bputs = url_bio_bputs,
> >>> +    .bgets = NULL,
> >>> +    .ctrl = url_bio_ctrl,
> >>> +    .create = url_bio_create,
> >>> +    .destroy = url_bio_destroy,
> >>> +};
> >>> +
> >>>  #endif
> >>>  #include "network.h"
> >>>  #include "os_support.h"
> >>> @@ -148,6 +239,9 @@ static int tls_open(URLContext *h, const char *uri, 
> > int flags, AVDictionary **op
> >>>      struct addrinfo hints = { 0 }, *ai = NULL;
> >>>      const char *proxy_path;
> >>>      int use_proxy;
> >>> +#if CONFIG_OPENSSL
> >>> +    BIO *bio;
> >>> +#endif
> >>>
> >>>      ff_tls_init();
> >>> 
> >>> @@ -220,8 +314,12 @@ static int tls_open(URLContext *h, const char *uri, 
> > int flags, AVDictionary **op
> >>>          }
> >>>      }
> >>>      gnutls_credentials_set(c->session, GNUTLS_CRD_CERTIFICATE, c->cred);
> >>> +    c->tcp->flags |= AVIO_FLAG_NONBLOCK;
> >>> +    gnutls_transport_set_lowat(c->session, 0);
> >>> +    gnutls_transport_set_pull_function(c->session, gnutls_url_pull);
> >>> +    gnutls_transport_set_push_function(c->session, gnutls_url_push);
> >>>      gnutls_transport_set_ptr(c->session, (gnutls_transport_ptr_t)
> >>> -                                         (intptr_t) c->fd);
> >>> +                                         (intptr_t) c->tcp);
> >>
> >> The intptr_t extra cast can probably be removed.
> >
> > Indeed, both casts can be removed - done locally.
> 
> Also, I take this review as a "the rest of this looks good to me except 
> for this detail".

Yep.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to