Hi, On Sun, Nov 20, 2016 at 9:16 AM, Michael Niedermayer <mich...@niedermayer.cc > 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 > <mich...@niedermayer.cc > > > 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? Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel