[MPlayer-dev-eng] [PATCH] -force-key-frames option

Nicolas George nicolas.george at normalesup.org
Sun Oct 17 13:41:36 CEST 2010


Le sextidi 26 vendémiaire, an CCXIX, Reimar Döffinger a écrit :
> According to the documentation it is wrong to assume whether %n increases
> the sscanf value or not.

AFAIK, both C99 and Single Unix agree that "Execution of a %n directive
does not increment the assignment count returned at the completion of
execution of the fscanf function."

If there are known broken systems that need to be worked around, I can just
rewrite it with ">= 3" instead of "== 3".

> Returning the parsed length would avoid the cast.

Done.

> Also I'd suggest to consider making this the extra argument,
> and if it is 0 also check that the end is the terminating 0
> or otherwise return an error.

Done a slightly different way: a third "endchar" argument, and return an
error if the end of the string is neither 0 nor endchar. This is not the
most generic API, but it is simple and it works for all the current cases;
we can not predict what the next use will actually do.

> Currently our parsing code seems to accept e.g. "2 centuries" as 2 seconds.

Corrected as a consequence of the above.

> Actually you could just make the parsing function check that there were
> no extra characters (e.g. by adding a %c at the end to the parse string
> and check nothing gets assigned to it), and then use strsep to parse
> the string.

strsep is not standard, and I do not like the idea of changing the input
string unnecessarily.

> valgrind won't like that this is never freed.

Added some free.

> I still don't see the point of munging assignments into ifs.
> IMO it is really bad for readability.

Changed.

Patch 03, 04 and 05 are unchanged.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-forcekeyframes-20101017-1331-01-parser.diff
Type: text/x-diff
Size: 2254 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101017/c69d7ff6/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-forcekeyframes-20101017-1331-02-option.diff
Type: text/x-diff
Size: 3795 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101017/c69d7ff6/attachment-0001.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-forcekeyframes-20101017-1331-03-lavc.diff
Type: text/x-diff
Size: 1253 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101017/c69d7ff6/attachment-0002.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-forcekeyframes-20101017-1331-04-x264.diff
Type: text/x-diff
Size: 1050 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101017/c69d7ff6/attachment-0003.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-forcekeyframes-20101017-1331-05-xvid.diff
Type: text/x-diff
Size: 1190 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101017/c69d7ff6/attachment-0004.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101017/c69d7ff6/attachment.pgp>


More information about the MPlayer-dev-eng mailing list