[FFmpeg-devel] [PATCH 2/4] parse_key_value_pair: Support keys without values with AV_DICT_ALLOW_NULLVAL

[Michael Niedermayer]

On Sun, Jul 21, 2013 at 01:27:31PM +0200, Stefano Sabatini wrote:
> "parse_key_value_pair" is not useful as module name, especially given
> we have different implementations with different levels of NIH-ness.
> 

> Also any reason this is not merged with the previous patch?

improves git bisect usefullness if theres a bug


> 
> On date Saturday 2013-07-20 18:55:03 +0200, Michael Niedermayer encoded:
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavutil/dict.c |   12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/libavutil/dict.c b/libavutil/dict.c
> > index f965492..40434b6 100644
> > --- a/libavutil/dict.c
> > +++ b/libavutil/dict.c
> > @@ -116,6 +116,7 @@ static int parse_key_value_pair(AVDictionary **pm, const char **buf,
> >                                  const char *key_val_sep, const char *pairs_sep,
> >                                  int flags)
> >  {
> 
> > +    char *buf2= *buf;
> 
> Nit: *buf2 = *buf;
> 
> Who gives a bit about vertical align anyway.
> 
> >      char *key = av_get_token(buf, key_val_sep);
> >      char *val = NULL;
> >      int ret;
> > @@ -125,8 +126,19 @@ static int parse_key_value_pair(AVDictionary **pm, const char **buf,
> >          val = av_get_token(buf, pairs_sep);
> >      }
> >  
> 
> > +    if (flags & AV_DICT_ALLOW_NULLVAL) {
> > +        char *key2 = av_get_token(&buf2, pairs_sep);
> > +        if (!key || (key2 && strlen(key2) < strlen(key))) {
> > +            FFSWAP(char *, *buf, buf2);
> > +            FFSWAP(char *, key, key2);
> > +            av_freep(&val);
> > +        }
> > +        av_freep(&key2);
> > +    }
> 
> What's the final outcome of this hack?

allowing ommiting values, that is a key without value. and that then
results in a null value 


> 
> >      if (key && *key && val && *val)
> >          ret = av_dict_set(pm, key, val, flags);
> > +    else if (key && *key && !val && (flags & AV_DICT_ALLOW_NULLVAL))
> > +        ret = av_dict_set(pm, key, val, flags);
> >      else
> >          ret = AVERROR(EINVAL);
> 
> The result is that now we have several option list functions with
> different behaviours (av_dict_parse_string(), av_opt_get_key_value(),
> av_opt_set_from_string()). Ideally we should try to unify them, rather
> than adding more arbitrary differences.

The current stuation is that libx264 has 2 slightly incompatible
option syntaxes. Id like to fix that.
Iam somewhat reluctant to change other unrelated functions and add
dead codepathes to them. (dead because nothing would set
AV_DICT_ALLOW_NULLVAL for them) 

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130721/1cb60a66/attachment.asc>