New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PiCameraCircularIO missing data due to invalid timestamps #357

Closed
dbbnrl opened this Issue Jan 9, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@dbbnrl
Contributor

dbbnrl commented Jan 9, 2017

I've been having problems with copy_to() writing out no data even when my circular buffer is quite large. After some digging, it appears the problem is this:

  • PiCameraCircularIO._find_seconds() does not find any sts header frames in the required time range, because it encounters a timestamp with a very large negative value, causing the break statement to trigger. This negative value is the "unknown time" value referenced in the docs, (MMAL_TIME_UKNOWN). But not exactly...
  • The timestamp is created in PiVideoEncoder._callback_write(), where it is first compared against mmal.MMAL_TIME_UKNOWN to avoid exactly this situation, but...
  • The comparison fails because buf.pts is not a c_long!

It's easy to see the issue:

>>> import ctypes as ct
>>> bad = ct.c_int64(1<<63)
>>> bad
c_long(-9223372036854775808)
>>> x = -9223372036854775808
>>> x == bad
False

I'm unfamiliar enough with the code to know exactly how this should be handled yet. Is buf.pts expected to be a c_long? I'll do a bit more research and then submit a PR, but I thought I'd document what I have so far here.

@dbbnrl

This comment has been minimized.

Contributor

dbbnrl commented Jan 9, 2017

OK, so if I add these two lines to MMALBuffer._get_pts() in mmalobj.py:

        print(type(self._buf[0]))
        print(type(self._buf[0].pts))

I see:

<class 'picamera.mmal.MMAL_BUFFER_HEADER_T'>
<class 'int'>

This is not the result I would have expected, but I have no experience with Python's ctypes.

Wheee! Isn't debugging dynamically typed languages fun!

@dbbnrl

This comment has been minimized.

Contributor

dbbnrl commented Jan 9, 2017

Last comment before I wait for more knowledgeable people to chime in:

If I modify the code in PiVideoEncoder._callback_write() to extract the ctypes 'value' field as follows:

            timestamp=
                None
                if buf.pts in (0, mmal.MMAL_TIME_UNKNOWN.value) else
                buf.pts,

... everything seems to work.

@helmutka

This comment has been minimized.

helmutka commented Jan 9, 2017

Seems to be the same issue as #302 and #319 but no solution since half a year.

dbbnrl added a commit to dbbnrl/picamera that referenced this issue Jan 9, 2017

@waveform80

This comment has been minimized.

Owner

waveform80 commented Jan 12, 2017

@dbbnrl nice bit of debugging there! This has been on the list a while, but it's been repeatedly shoved to the bottom of the pile by new feature development (largely mmalobj). Your surmise is absolutely correct: it should be using .value there otherwise it's just comparing object identity (oops!) - it's one of the more annoying aspects of ctypes (although there is a good reason for it).

I'm just going to check if I can work around this a different way: by using a straight-forward Python int instead of a ctypes.c_int64 because, if I can, that'll be more friendly for anyone wanting to use the mmalobj layer in future (it's largely intended to avoid much of the ctypes horror) and I'm anticipating with the "Python MMAL components" stuff in the new version that might be rather popular. Nonetheless, I'm very grateful for your efforts: it's always really nice when someone solves a problem for you!

@waveform80 waveform80 added the bug label Jan 12, 2017

@waveform80 waveform80 added this to the 1.13 milestone Jan 12, 2017

@dbbnrl

This comment has been minimized.

Contributor

dbbnrl commented Jan 13, 2017

Excellent, glad I could help. PiCamera is a joy to use!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment