rpicamsrc: fix nal alignment of output buffers

We claim output buffers are nal-aligned, but that wasn't
actually true: We would push out a partial nal in case
the nal doesn't fit into the max encoder-selected output
buffer size, and then the next buffer would not start
with a sync marker. That's not right and makes h264parse
unhappy.

Instead accumulate buffers until we have a full frame
(we can't rely on the NAL_END flag, it's always set).

Fixes #768

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/682>
This commit is contained in:
Tim-Philipp Müller 2020-07-24 16:14:00 +01:00
parent 477eba2dd7
commit 9588108d6e
3 changed files with 41 additions and 16 deletions

View file

@ -971,11 +971,11 @@ raspi_capture_fill_buffer(RASPIVID_STATE *state, GstBuffer **bufp,
if (runtime >= offset) if (runtime >= offset)
gst_pts = runtime - offset; gst_pts = runtime - offset;
} }
GST_LOG ("Buf (uS) PTS %" G_GINT64_FORMAT " DTS %" G_GINT64_FORMAT GST_LOG ("Buf %05u bytes FLAGS 0x%05x (uS) PTS %" G_GINT64_FORMAT
" STC %" G_GINT64_FORMAT " (latency %" G_GINT64_FORMAT " DTS %" G_GINT64_FORMAT " STC %" G_GINT64_FORMAT
"uS) TS %" GST_TIME_FORMAT, " (latency %" G_GINT64_FORMAT "uS) TS %" GST_TIME_FORMAT,
buffer->pts, buffer->dts, param.value, param.value - buffer->pts, buffer->length, buffer->flags, buffer->pts, buffer->dts, param.value,
GST_TIME_ARGS (gst_pts)); param.value - buffer->pts, GST_TIME_ARGS (gst_pts));
} }
else { else {
GST_LOG ("use-stc=false. Not applying STC to buffer"); GST_LOG ("use-stc=false. Not applying STC to buffer");
@ -988,7 +988,12 @@ raspi_capture_fill_buffer(RASPIVID_STATE *state, GstBuffer **bufp,
GST_BUFFER_DTS(buf) = GST_BUFFER_PTS(buf) = gst_pts; GST_BUFFER_DTS(buf) = GST_BUFFER_PTS(buf) = gst_pts;
/* FIXME: Can we avoid copies and give MMAL our own buffers to fill? */ /* FIXME: Can we avoid copies and give MMAL our own buffers to fill? */
gst_buffer_fill(buf, 0, buffer->data, buffer->length); gst_buffer_fill(buf, 0, buffer->data, buffer->length);
ret = GST_FLOW_OK;
/* NAL_END is bogus and can't be trusted */
if ((buffer->flags & MMAL_BUFFER_HEADER_FLAG_FRAME_END))
ret = GST_FLOW_OK;
else
ret = GST_FLOW_KEEP_ACCUMULATING;
} }
mmal_buffer_header_mem_unlock(buffer); mmal_buffer_header_mem_unlock(buffer);

View file

@ -67,6 +67,7 @@ GST_DEBUG_CATEGORY_EXTERN (gst_rpi_cam_src_debug);
#define vcos_log_warn GST_WARNING #define vcos_log_warn GST_WARNING
#define GST_FLOW_ERROR_TIMEOUT GST_FLOW_CUSTOM_ERROR #define GST_FLOW_ERROR_TIMEOUT GST_FLOW_CUSTOM_ERROR
#define GST_FLOW_KEEP_ACCUMULATING GST_FLOW_CUSTOM_SUCCESS
G_BEGIN_DECLS G_BEGIN_DECLS

View file

@ -1480,16 +1480,32 @@ gst_rpi_cam_src_create (GstPushSrc * parent, GstBuffer ** buf)
} }
g_mutex_unlock (&src->config_lock); g_mutex_unlock (&src->config_lock);
/* FIXME: Use custom allocator */ *buf = NULL;
ret = raspi_capture_fill_buffer (src->capture_state, buf, clock, base_time);
if (*buf) { do {
GST_LOG_OBJECT (src, "Made buffer of size %" G_GSIZE_FORMAT, GstBuffer *cbuf = NULL;
gst_buffer_get_size (*buf));
/* Only set the duration when we have a PTS update from the rpi encoder. /* FIXME: Use custom allocator */
* not every buffer is a frame */ ret =
if (GST_BUFFER_PTS_IS_VALID (*buf)) raspi_capture_fill_buffer (src->capture_state, &cbuf, clock, base_time);
GST_BUFFER_DURATION (*buf) = src->duration;
} if (cbuf != NULL) {
GST_LOG_OBJECT (src, "Made buffer of size %" G_GSIZE_FORMAT,
gst_buffer_get_size (cbuf));
if (*buf == NULL) {
/* Only set the duration when we have a PTS update from the rpi encoder.
* not every buffer is a frame */
if (GST_BUFFER_PTS_IS_VALID (cbuf))
GST_BUFFER_DURATION (cbuf) = src->duration;
*buf = cbuf;
} else {
*buf = gst_buffer_append (*buf, cbuf);
}
cbuf = NULL;
}
} while (ret == GST_FLOW_KEEP_ACCUMULATING);
if (ret == GST_FLOW_ERROR_TIMEOUT) { if (ret == GST_FLOW_ERROR_TIMEOUT) {
GST_ELEMENT_ERROR (src, STREAM, FAILED, GST_ELEMENT_ERROR (src, STREAM, FAILED,
@ -1498,6 +1514,9 @@ gst_rpi_cam_src_create (GstPushSrc * parent, GstBuffer ** buf)
ret = GST_FLOW_ERROR; ret = GST_FLOW_ERROR;
} }
if (ret != GST_FLOW_OK)
gst_clear_buffer (buf);
if (clock) if (clock)
gst_object_unref (clock); gst_object_unref (clock);
return ret; return ret;