[FFmpeg-devel] [PATCH] Add Apple HTTP Live Streaming protocol handler

[Martin Storsjö]

On Tue, 17 Aug 2010, Michael Niedermayer wrote:

> On Tue, Aug 17, 2010 at 04:15:20PM +0300, Martin Storsj? wrote:
> > On Thu, 5 Aug 2010, Ronald S. Bultje wrote:
> > 
> > > On Thu, Aug 5, 2010 at 4:09 PM, Martin Storsj? <martin at martin.st> wrote:
> > > > On Wed, 4 Aug 2010, Ronald S. Bultje wrote:
> > > >> On Tue, Jul 27, 2010 at 5:45 AM, Martin Storsj? <martin at martin.st> wrote:
> > > >> [..]
> > > >> > -static void handle_basic_params(HTTPAuthState *state, const char *key,
> > > >> > +static void handle_basic_params(void *context, const char *key,
> > > >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int key_len, char **dest, int *dest_len)
> > > >> > ?{
> > > >> > + ? ?HTTPAuthState *state = context;
> > > >> > ? ? ?if (!strncmp(key, "realm=", key_len)) {
> > > >> > ? ? ? ? ?*dest ? ? = ? ? ? ?state->realm;
> > > >> > ? ? ? ? ?*dest_len = sizeof(state->realm);
> > > >> > ? ? ?}
> > > >> > ?}
> > > >> >
> > > >> > -static void handle_digest_params(HTTPAuthState *state, const char *key,
> > > >> > +static void handle_digest_params(void *context, const char *key,
> > > >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int key_len, char **dest, int *dest_len)
> > > >> > ?{
> > > >> > + ? ?HTTPAuthState *state = context;
> > > >> > ? ? ?DigestParams *digest = &state->digest_params;
> > > >> >
> > > >> > ? ? ?if (!strncmp(key, "realm=", key_len)) {
> > > >> > @@ -116,9 +62,10 @@ static void handle_digest_params(HTTPAuthState *state, const char *key,
> > > >> > ? ? ?}
> > > >> > ?}
> > > >> >
> > > >> > -static void handle_digest_update(HTTPAuthState *state, const char *key,
> > > >> > +static void handle_digest_update(void *context, const char *key,
> > > >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int key_len, char **dest, int *dest_len)
> > > >> > ?{
> > > >> > + ? ?HTTPAuthState *state = context;
> > > >> > ? ? ?DigestParams *digest = &state->digest_params;
> > > >> >
> > > >> > ? ? ?if (!strncmp(key, "nextnonce=", key_len)) {
> > > >>
> > > >> These extra lines should be avoidable by casting the function to a
> > > >> proper type (a typedef helps here) when passing it to the function.
> > > >> Other than that, this patch is OK, feel free to apply once #2 is OK'ed
> > > >> also (since without it, this isn't needed).
> > > >
> > > > I tried doing this, but I'm not sure I agree it's prettier. The attached
> > > > patch is the diff compared to the previous version. What do you think?
> > > 
> > > I personally like it a lot better, but that's just me. If others
> > > disagree violently, then let's not...
> > 
> > Does anyone else have an opinion on this? That is, of these two variants, 
> > which one do you prefer:
> > 
> > static void handle_basic_params(void *context, const char *key,
> >                                 int key_len, char **dest, int *dest_len)
> > {
> >     HTTPAuthState *state = context;
> >     if (!strncmp(key, "realm=", key_len)) {
> >         *dest     =        state->realm;
> >         *dest_len = sizeof(state->realm);
> >     }
> > }
> > 
> > ff_parse_key_value(p, handle_basic_params, state);
> > 
> > Or:
> > 
> > static void handle_basic_params(HTTPAuthState *state, const char *key,
> >                                 int key_len, char **dest, int *dest_len)
> > {
> >     if (!strncmp(key, "realm=", key_len)) {
> >         *dest     =        state->realm;
> >         *dest_len = sizeof(state->realm);
> >     }
> > }
> > 
> > ff_parse_key_value(p, (ff_parse_key_val_cb) handle_basic_params, state);
> > 
> > 
> > That is, keeping the context pointer as an opaque void pointer that is 
> > cast within the callback function, or casting the whole function pointer 
> > using a typedef?
> 
> iam definitly prefering the handle_basic_params(HTTPAuthState *state

Ok, thanks for voicing your opinion, that form it is then.

// Martin