[MPlayer-dev-eng] [PATCH] Fractional part of time stamp in OSD
nicolas.george at normalesup.org
Fri Jan 28 17:45:12 CET 2011
Le nonidi 9 pluviôse, an CCXIX, Christian a écrit :
> Attached patch allows to optionally also show the fractional part of the
> time stamp in the OSD.
Seems potentially useful.
> +.B \-osd-fractions <0\-2>
You forgot that one.
> +show approximated frame count within current second. This frame count is not
> +accurate but only an approximation. For variable fps, the approximation is
> +known to be far off the correct frame count.
I think the rule is tu start sentences on new lines.
> Setzt die Anzeigedauer der OSD-Meldungen in ms (Standard: 1000).
My German is too rusty for me to give advice on that part.
> + snprintf(fractions_text, 5, "%4.2f", mpctx->sh_video->pts - pts);
Potential desync between one part of the code and the other. Better use
> + fractions_text_start++; //This is to skip beyond the leading 0 in eg 0.12
That seems awkward. Couldn't you use "%02d" and
(int)(diff * 100 + 0.5) % 100?
> + } else if (osd_fractions==2)
> + //print fractions by estimating the frame count within the second
> + //
> + //rounding or cutting off numbers after the decimal point causes problems
> + //because of float's precision and movies, whose first frame is not exactly at timestamp 0
> + //Therefore, we add 0.2 and cut off at the decimal point, which proved as good heuristic
> + snprintf(fractions_text, 4, ".%02d", (int) ( ( mpctx->sh_video->pts - pts ) * mpctx->sh_video->fps + 0.2 ) );
Nitpick: your lines are too long, I had to enlarge my xterm to read them
comfortably. Also, I fear a multiline comment before a brace-less if/else
could lead to future bugs (people see several indented lines, they assume
they are grouped by braces).
But I grant you that the surrounding code is not a perfect example of
readability to start with.
Apart from that, it seems fine.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 198 bytes
Desc: Digital signature
More information about the MPlayer-dev-eng