Re-work preservation of timestamps

We don't need all those extra encoder fields, and it'd be nice if the
split_recording method could return the entire frame meta-data rather
than just the timestamp.

This also re-works "None" timestamps in general. We can reasonably
assume the camera's clock never goes backward during a recording so
in the event of a "None" timestamp we should *always* repeat the
last timestamp.

Author: waveform80

picamera/camera.py

@@ -1086,6 +1086,9 @@ def split_recording(self, output, splitter_port=1, **options):
         and *inline_headers* must be ``True`` when :meth:`start_recording` is
         called (this is the default).
 
+        The method returns the meta-data of the first :class:`PiVideoFrame`
+        that is written to the new output.
+
         .. versionchanged:: 1.3
             The *splitter_port* parameter was added
 

picamera/encoders.py

@@ -600,11 +600,8 @@ def __init__(
         super(PiVideoEncoder, self).__init__(
                 parent, camera_port, input_port, format, resize, **options)
         self._next_output = []
+        self._split_frame = None
         self.frame = None
-        self.chunk_timestamp = None
-        self.next_pts = 0
-        self.last_pts = 0
-        self.split_pts = 0
 
     def _create_encoder(
             self, format, bitrate=17000000, intra_period=None, profile='high',
@@ -811,7 +808,6 @@ def start(self, output, motion_output=None):
                 timestamp=0,
                 complete=False,
                 )
-        self.chunk_timestamp = None
         if motion_output is not None:
             self._open_output(motion_output, PiVideoFrameType.motion_data)
         super(PiVideoEncoder, self).start(output)
@@ -849,7 +845,7 @@ def split(self, output, motion_output=None):
         # intra_period / framerate gives the time between I-frames (which
         # should also coincide with SPS headers). We multiply by three to
         # ensure the timeout is deliberately excessive, and clamp the minimum
-        # timeout to 10 seconds (otherwise unencoded formats tend to fail
+        # timeout to 15 seconds (otherwise unencoded formats tend to fail
         # presumably due to I/O capacity)
         if self.parent:
             framerate = self.parent.framerate + self.parent.framerate_delta
@@ -861,37 +857,20 @@ def split(self, output, motion_output=None):
         if not self.event.wait(timeout):
             raise PiCameraRuntimeError('Timed out waiting for a split point')
         self.event.clear()
-        return self.split_pts
+        return self._split_frame
 
     def _callback_write(self, buf, key=PiVideoFrameType.frame):
         """
         Extended to implement video frame meta-data tracking, and to handle
         splitting video recording to the next output when :meth:`split` is
         called.
         """
-        timestamp = (
-            None
-            if buf.pts in (0, mmal.MMAL_TIME_UNKNOWN) else
-            buf.pts
-        )
-        complete = bool(buf.flags & mmal.MMAL_BUFFER_HEADER_FLAG_FRAME_END)
-        # Uncomment to observe skipped frames:
-        # if timestamp:
-        #     while self.next_pts and timestamp > self.next_pts:
-        #         print("<SKIP!>")
-        #         self.next_pts += 1000000 / self.parent.framerate
-        #     self.next_pts = timestamp + 1000000 / self.parent.framerate
-        if timestamp is None:
-            timestamp = self.chunk_timestamp
-        if complete:
-            self.chunk_timestamp = None
-        else:
-            self.chunk_timestamp = timestamp
-        self.frame = PiVideoFrame(
+        last_frame = self.frame
+        this_frame = PiVideoFrame(
             index=
-                self.frame.index + 1
-                if self.frame.complete else
-                self.frame.index,
+                last_frame.index + 1
+                if last_frame.complete else
+                last_frame.index,
             frame_type=
                 PiVideoFrameType.key_frame
                 if buf.flags & mmal.MMAL_BUFFER_HEADER_FLAG_KEYFRAME else
@@ -902,18 +881,24 @@ def _callback_write(self, buf, key=PiVideoFrameType.frame):
                 PiVideoFrameType.frame,
             frame_size=
                 buf.length
-                if self.frame.complete else
-                self.frame.frame_size + buf.length,
+                if last_frame.complete else
+                last_frame.frame_size + buf.length,
             video_size=
-                self.frame.video_size
+                last_frame.video_size
                 if buf.flags & mmal.MMAL_BUFFER_HEADER_FLAG_CODECSIDEINFO else
-                self.frame.video_size + buf.length,
+                last_frame.video_size + buf.length,
             split_size=
-                self.frame.split_size
+                last_frame.split_size
                 if buf.flags & mmal.MMAL_BUFFER_HEADER_FLAG_CODECSIDEINFO else
-                self.frame.split_size + buf.length,
-            timestamp=timestamp,
-            complete=complete,
+                last_frame.split_size + buf.length,
+            timestamp=
+                # Time cannot go backwards, so if we've got an unknown pts
+                # simply repeat the last one
+                last_frame.timestamp
+                if buf.pts in (0, mmal.MMAL_TIME_UNKNOWN) else
+                buf.pts,
+            complete=
+                bool(buf.flags & mmal.MMAL_BUFFER_HEADER_FLAG_FRAME_END)
             )
         if self._intra_period == 1 or (buf.flags & mmal.MMAL_BUFFER_HEADER_FLAG_CONFIG):
             with self.outputs_lock:
@@ -926,21 +911,20 @@ def _callback_write(self, buf, key=PiVideoFrameType.frame):
                     self._close_output(new_key)
                     self._open_output(new_output, new_key)
                     if new_key == PiVideoFrameType.frame:
-                        self.frame = PiVideoFrame(
-                                index=self.frame.index,
-                                frame_type=self.frame.frame_type,
-                                frame_size=self.frame.frame_size,
-                                video_size=self.frame.video_size,
+                        this_frame = PiVideoFrame(
+                                index=this_frame.index,
+                                frame_type=this_frame.frame_type,
+                                frame_size=this_frame.frame_size,
+                                video_size=this_frame.video_size,
                                 split_size=0,
-                                timestamp=self.frame.timestamp,
-                                complete=self.frame.complete,
+                                timestamp=this_frame.timestamp,
+                                complete=this_frame.complete,
                                 )
-                self.split_pts = self.last_pts
+                self._split_frame = this_frame
                 self.event.set()
         if buf.flags & mmal.MMAL_BUFFER_HEADER_FLAG_CODECSIDEINFO:
             key = PiVideoFrameType.motion_data
-        if self.frame.timestamp:
-            self.last_pts = self.frame.timestamp
+        self.frame = this_frame
         return super(PiVideoEncoder, self)._callback_write(buf, key)
 
 

picamera/frames.py

@@ -150,10 +150,9 @@ class PiVideoFrame(namedtuple('PiVideoFrame', (
         .. warning::
 
             Currently, the camera occasionally returns "time unknown" values in
-            this field which picamera represents as ``None``. If you are
-            querying this property you will need to check the value is not
-            ``None`` before using it. This happens for SPS header "frames",
-            for example.
+            this field. In this case, picamera will simply re-use the timestamp
+            of the previous frame (under the assumption that time never goes
+            backwards). This happens for SPS header "frames", for example.
 
     .. attribute:: complete
 
@@ -213,4 +212,3 @@ def header(self):
                 'PiVideoFrame.frame_type for equality with '
                 'PiVideoFrameType.sps_header instead'))
         return self.frame_type == PiVideoFrameType.sps_header
-