GNOME Bugzilla – Bug 685000
ValueError : 18446744073709551615 no in range 0 to 18446744073709551615
Last modified: 2012-10-12 09:07:51 UTC
Created attachment 225291 [details] [review] Do not trigger value error on big guint64 value (here max guint64) In gstreamer 1.0 , Gst.CLOCK_TIME_NONE is 18446744073709551615 (ie max guint64). I got "ValueError : 18446744073709551615 no in range 0 to 18446744073709551615" passing it to the methods. This issue (the range is inclusive and indeed 18446744073709551615 ie 0xFFFFFFFFFFFFFFFF is max uint64. Though there is: in _pygi_marshal_from_py_uint64 in this special case , there is "gint64 long_ = (gint64) PyInt_AsLong (py_long);" then it is checked for "< 0" then assigned as guint64 if not failed obviously for max guint64 the value is negative thus one get a ValueError Attached patch fixes that by checking the guint64 against its max value instead of casting to gint64 then checking if positive.
This is supposed to be covered by the test_gi.TestUInt64.test_uint64_in test case, which runs fine on x86, x86_64, ppc64, and a lot of other platforms known to me, with python 2.6.8 and python 2.7.3 (3.x isn't affected by this code path at all). In this case, PyInt_Check (py_long) is false, as py_long is 18446744073709551615L, i. e. a Python long which doesn't fit into a Python int any more. However, I can reproduce this with $ python >>> from gi.repository import Gst >>> Gst.Event.new_segment_done(0, Gst.BUFFER_OFFSET_NONE) Traceback (most recent call last):
+ Trace 230928
return info.invoke(cls, *args, **kwargs)
Ah no, that's wrong -- this is expecting a gint64. The broken error message is tracked in bug 684314.
For an uint64 Gst method I tried: $ PYTHONPATH=. python -c 'from gi.repository import Gst; print Gst.Event.new_step(Gst.Format.DEFAULT, Gst.BUFFER_OFFSET_NONE, 1.0, False, False)' (process:27765): GStreamer-CRITICAL **: gst_structure_new_id: assertion `name_quark != 0' failed which crashes way later in the game. gdb'ing it shows that in _pygi_marshal_from_py_uint64() it also fails the PyInt_Check() check. On what platform are you running this? I don't see this on at least x86_64 and powerpc64.
Created attachment 225381 [details] testcase for guint64 as "-1" my bad. This is on x86_64 though the issue was pipeline.get_state(-1) . $ python getstate-clocktimenone.py GStreamer 1.0.0 (GIT) Traceback (most recent call last):
+ Trace 230932
ret = pipeline.get_state(-1)
return info.invoke(*args, **kwargs)
seems it works well with Gst.CLOCK_TIME_NONE instead of "-1" . But in my test with pygobject from last week both were failing, so I thought both were the same issue.
For me it doesn't even work with Gst.CLOCK_TIME_NONE, as that is -1 as well: >>> print Gst.CLOCK_TIME_NONE -1 At least with trunk I now get the correct error message: ValueError: -1 not in range 0 to 18446744073709551615 Gst.ClockTime is an alias for guint64, so it doesn't accept negative values. Before, the error message was confusing, so that part is fixed. The GIR defines the constant correctly: <constant name="CLOCK_TIME_NONE" value="18446744073709551615" c:type="GST_CLOCK_TIME_NONE"> <type name="ClockTime" c:type="GstClockTime"/> </constant> So I think that's the bug we need to fix.
Ah, ignore me. I was looking at Gst-1.0.gir (where the definition is correct), but the program used the Gst 0.10 typelibs, which indeed define CLOCK_TIME_NONE wrongly: <constant name="CLOCK_TIME_NONE" value="-1" c:type="GST_CLOCK_TIME_NONE"> <type name="gint" c:type="gint"/> </constant> With Gst 1.0, the constant is correct: >>> Gst.CLOCK_TIME_NONE 18446744073709551615 And I can run your program just fine with GStreamer 1.0 and trunk, when I replace -1 with Gst.CLOCK_TIME_NONE. So once again I'm not sure what the bug is here, and how to trigger the code path that you changed with the patch. (See comment 1, it should be irrelevant). Thanks!
the bug could be marked wontfix . Ie the issue is that -1 is used as an alias for guint64 ax value in a few gst programs (even 1.0) . But if the issue is marked as wontfix it is fine too.
Comment on attachment 225291 [details] [review] Do not trigger value error on big guint64 value (here max guint64) I applied this patch because it's a nice stylistic improvement in the range check for python 2. However, please note that computation wise it is pretty much equivalent, and it does not change the ValueError that you get in the slightest.
I set this as "wont fix" as I think correctness here is better and more in line with standard Python behaviour, i. e. not accepting negative values (and transparently turning them into uint64) for unsigned data types. Thanks!
Hm, that commit was bad; we actually do need to check for < 0 here; PyInt_AsLong() does return a "long", not an unsigned type, so that commit broke raising a proper ValueError when trying to pass a negative value to an usigned argument. I didn't notice that before because I missed running the tests with Python 2 back then. Fixed in http://git.gnome.org/browse/pygobject/commit/?id=a2ab72aa39824579d1767d1fdba7e1031341f86c