[FFmpeg-devel] Patch review: av_dict: add support for empty values

[Michael Niedermayer]

On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote:
> I've made some patches, and am finally getting around to sending them
> upstream.
> 
> I'm trying out gmail for dealing with a high-traffic list like this one,
> instead of my usual mutt.  Are the one-line-paragraph long lines it's
> probably going to produce an annoyance for anyone?  I didn't notice
> anything about line lengths in emails in
> https://www.ffmpeg.org/developer.html
> 
> I put in pull requests on github for 4 different branches.  Should I send
> email to the list for each set of patches?  Should I git-send-email, or is
> a mail with a URL for the github pull request enough?  (Or is just a pull
> request enough?  developer.html seems to say that sending a pull request is
> an alternative to mailing patches to the list.)
> 
> 
> Anyway, the av_dict change is the one that requires the most review, so
> I'll make this email focus on that set of changes.
> https://github.com/FFmpeg/FFmpeg/pull/118
> 
>   Currently, -x264-params nocabac:ref=4:ssim  isn't supported.  You have to
> say nocabac=1:ref=4:ssim=1  (or cabac=0...).  (and yes, I know about
> -preset, but superfast no-cabac is useful... anyway.)
> 
>  Also, libx265.c is very minimal, and needs a lot of things passed in
> -x265-params.  I didn't fix that, but did make it support keys with no
> value.
> 
> libx264 has two different ways of passing option strings (-x264-params and
> -x264opts), due to avconv gratuitously renaming things.  The insane part is
> that the two strings were parsed differently.  One was handled by av_dict,
> the other with some custom code that didn't support quoting.
> 
> In my x26x-opts branch (which follows the av_dict branch), I add both
> option strings to the same av_dict.  (x264opts and then x264-params, in
> that order, regardless of order on the command line, same as before).  I
> don't think it's ever useful to use the same key multiple times.  Tune only
> works as -tune ssim, not in x264-params, and you can use a comma-separated
> list for -tune animation,ssim.
> 
> There was a previous attempt at keys with no value that didn't get merged:
>  http://ffmpeg.org/pipermail/ffmpeg-devel/2013-July/146329.html
> 
> I should borrow the idea of adding a new flag for supporting NULL as a
> value, instead of the cumbersome empty-string method I went with to
> minimize changes to some parts of the code.  Having a flag would make sure
> the behaviour didn't change for any callers that weren't expecting it.
> (i.e. it would still reject key:key).
>

> So are any of these changes useful?  I assume I'll end up redoing the
> commits anyway after a review, so I didn't try to make a final
> ready-for-commit version yet.  (For that, I would merge the bugfix changes
> into my original buggy commit.)  For review, I'd suggest looking at the
> diff of master:av_dict.  The 3 commits after that changing libx264.c and
> libx265.c should each stand on their own.  (And actually the libx264.c one
> should be split into a couple commits.)
> 
> Anyway, I wanted to post first and get some feedback before spending more
> time on it.

to awnser your question
i definitly think the changes are usefull


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150304/75e01f60/attachment.asc>