Hi ----- Original Message ----- > > > > Hi > > > > ----- Original Message ----- > > > > > > > > > > > > > > 3 - 4294967295 == 4 is not a portable assumption. Neither is > > > > > > > (uint32_t) 3 - (uint32_t) 4294967295 == 4 > > > > > > > > > > > > Ok, I am not an expert in portability, do you have an example where > > > > > > this > > > > > > would give different results? > > > > > > > > > > > > > > > > If int is 64 bit (uint32_t) 3 - (uint32_t) 4294967295 == -4294967292. > > > > > It seems weird but is due to the integral promotion rules. > > > > > > > > > > > > According to C standard, there is no integer promotion when both > > > > operands > > > > are > > > > of the same type: > > > > http://c0x.coding-guidelines.com/6.3.1.8.html: > > > > 712 If both operands have the same type, then no further conversion is > > > > needed. > > > > > > > > > > You are skipping 710 and 711. > > > > right, but then all platforms we care about use 32bit int afaik, so no > > promotion occurs. I think we could add a strict check in configure, that > > would make it clear. Then we don't have to half the max delay. But couldn't > > we find a more solid solution to handle time warp, by remembering the last > > time? > > > > Yes, currently all platforms we support have int of 32 bit and ILP64/SILP64 > platforms are quite rare however being client is more likely to require > a port to new systems in the future. > Usually integers are not supposed to overflow (if an error does not happen) > so it's not a problem to port to such platforms unless, like in this case, > you have overflows. I don't see much issue to maintain the patch I proposed, > is enough documented (and if not comments can be added) and not so big. > Creating a monotonic multimedia time can be a bit challenging as the time > can drift a bit so potentially you could have sequences like 1, 5, 2, 8, 11. > The time depends on a server monotonic timer and a client monotonic one, > if you have large drifts (days) you are having some severe system bugs in > either the server or the client. In case of migration the reference get > updated so it's not an issue (the frame timers get dropped during VM > migration), > in case of suspend/resume the client get disconnected and the timers on > the server frames get dropped. > I actually was thinking you were joking about the 42/24 days... why would > we need such constraint to require such changes? This fix is quite theorical, so is the limitation or new constrains you add. I have 2 questions I would like to answer before accepting your change: - is there a better way to handle time warping without involving integer arithmetic and adding new constrains - is the current code good enough in case the arch has 32bit int? (no integer promotion) > > > > > > > > > > > > > Similar to > > > > > printf("%d\n", (uint16_t) 10 - (uint16_t) 16); > > > > > which give -6 (but maybe some people could point out that this result > > > > > is > > > > > not portable either... from my knowledge it is). > > > > > > > > > > > This would be a gcc bug then, right ? > > > I personally tested with different compilers too. Unless they all are > > > not following C rules integer promotion happens. > > > > > > > > > > > > > > > > > if time1 == UINT32_MAX and time2 == 3 I expect 4 while if > > > > > > > time2 == UINT32_MAX and time2 == 3 (this can happen for different > > > > > > > reasons) > > > > > > > I expect -4. This make computation easier. > > > > > > > > > > > > > > I'll add this example. > > > > > > > > > > > > Yeah, some test cases would be useful to understand the problem and > > > > > > the > > > > > > solution. > > > > > > > > > > > > > > > > Done > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > > ts before the last frame received to check if we overlapped, I > > > > > > > > think > > > > > > > > (otherwise we should consider this frame as a late frame) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Frediano > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel