On Sun, Nov 20, 2016 at 12:00:49PM -0500, Ronald S. Bultje wrote: > Hi, > > On Sun, Nov 20, 2016 at 9:16 AM, Michael Niedermayer <[email protected] > > wrote: > > > On Sun, Nov 20, 2016 at 08:56:15AM -0500, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Sun, Nov 20, 2016 at 6:57 AM, Michael Niedermayer > > <[email protected] > > > > wrote: > > > > > > > @@ -131,6 +132,20 @@ static int write_number(void *obj, const AVOption > > *o, > > > > void *dst, double num, int > > > > if (intnum == 1 && d == (double)INT64_MAX) *(int64_t *)dst = > > > > INT64_MAX; > > > > else *(int64_t *)dst = > > > > llrint(d) * intnum; > > > > break;} > > > > + case AV_OPT_TYPE_UINT64:{ > > > > + double d = num / den; > > > > + // We must special case uint64_t here as llrint() does not > > > > support values > > > > + // outside the int64_t range and there is no portable function > > > > which does > > > > + // "INT64_MAX + 1ULL" is used as it is representable exactly > > as > > > > IEEE double > > > > + // while INT64_MAX is not > > > > + if (intnum == 1 && d == (double)UINT64_MAX) { > > > > + *(int64_t *)dst = UINT64_MAX; > > > > + } else if (o->max > INT64_MAX + 1ULL && d > INT64_MAX + 1ULL) > > { > > > > + *(uint64_t *)dst = (llrint(d - (INT64_MAX + 1ULL)) + > > > > (INT64_MAX + 1ULL))*intnum; > > > > + } else { > > > > + *(int64_t *)dst = llrint(d) * intnum; > > > > + } > > > > + break;} > > > > case AV_OPT_TYPE_FLOAT: > > > > *(float *)dst = num * intnum / den; > > > > break; > > > > > > > > > For the stupid, like me: what does this do? More specifically, this seems > > > an integer codepath, but there is a double in there. Why? > > > > write_number() has a num/den double argument. If this is used to > > set a uint64_t to UINT64_MAX things fail as double cannot exactly > > represent UINT64_MAX. (the closest it can represent is "UINT64_MAX + 1" > > So it needs to be handled as a special case. Otherwise it turns into 0 > > > This sounds incredibly shaky. Is write_number() so fringe that we don't > need an integer codepath?
write_number() can take int64_t, double and 32/32bit rationals. Its done compactly by representing the value as a num/den * intnum for integer input num/den == 1 for double or rational input intnum == 1 doubles can represent int32 exactly so it can be used simplifying the code so there is a integer codepath basically Ive written this 11 years ago (860a40c8a7756a3e63e7f9bb0d9caaf742d26402 av_set_number()) which also means it had 11 years of testing on many platforms [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
