On Tue, 12 Apr 2011, Diego Biurrun wrote:

> On Tue, Apr 12, 2011 at 11:08:13AM +0300, Martin Storsjö wrote:
> > This can later be extended to support other AES bit sizes,
> > encryption, other crypto algorithms, reading the key from a URL, etc.
> > 
> > --- /dev/null
> > +++ b/libavformat/crypto.c
> > @@ -0,0 +1,170 @@
> > +/*
> > + * Decryption protocol handler
> > + * Copyright (c) 2011 Martin Storsjo
> 
> You skip the umlaut in your own name?

I normally do that within files, yes. Since Janne used the full umlaut 
version in the git author names during the git conversion, I've kept on 
using it there, though, for consistency.

> > +#define OFFSET(x) offsetof(CryptoContext, x)
> > +static const AVOption options[] = {
> > +{"key", "", OFFSET(key), FF_OPT_TYPE_BINARY },
> > +{"iv", "", OFFSET(iv), FF_OPT_TYPE_BINARY },
> > +{NULL}
> > +};
> > +static const AVClass crypto_class = {
> > +    "crypto", av_default_item_name, options, LIBAVUTIL_VERSION_INT
> > +};
> 
> That looks quite unreadable unindented and without separating empty
> lines.

Cleaned up locally. Normally, these AVOption lines get very very long, and 
skipping the indentation is a good compromise, but here it sure can be 
cleaned up.

> > +    if (av_strstart(uri, "crypto+", &nested_url)) {
> > +    } else if (av_strstart(uri, "crypto:", &nested_url)) {
> > +    } else {
> > +        av_log(NULL, AV_LOG_ERROR, "Unsupported url %s\n", uri);
> > +        ret = AVERROR(EINVAL);
> > +        goto err;
> > +    }
> 
> Two empty if blocks?

They indicate the two cases where the url was successfully parsed. 
Simplified into if (!av_strstart() && !av_strstart()) { error; }.

> > +        c->outptr += size;
> > +        c->outdata -= size;
> 
> extra good alignment karma awaits here

Wooh, extra karma for me \o/

> > +    c->outdata = BLOCKSIZE * blocks;
> > +    c->outptr = c->outbuffer;
> > +    c->indata_used += BLOCKSIZE * blocks;
> 
> ditto

Done

> > +        c->indata -= c->indata_used;
> > +        c->indata_used = 0;
> 
> ditto

Done

// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to