From c8c1c7f10fe7ee589a04b763db15415e48f8051d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 7 Jul 2011 12:23:24 +0200 Subject: [PATCH] omx: Add more checks to acquire_buffer() and return the current state additional to the buffer Also refactor the code flow in the video decoder for this. This makes the usage of acquire_buffer() easier and more atomic. --- omx/gstomx.c | 62 ++++++++++--- omx/gstomx.h | 9 +- omx/gstomxvideodec.c | 206 +++++++++++++++++++------------------------ 3 files changed, 152 insertions(+), 125 deletions(-) diff --git a/omx/gstomx.c b/omx/gstomx.c index f73d635dc0..1e3be61684 100644 --- a/omx/gstomx.c +++ b/omx/gstomx.c @@ -684,26 +684,43 @@ gst_omx_port_update_port_definition (GstOMXPort * port, return (err == OMX_ErrorNone); } -GstOMXBuffer * -gst_omx_port_acquire_buffer (GstOMXPort * port) +GstOMXAcquireBufferReturn +gst_omx_port_acquire_buffer (GstOMXPort * port, GstOMXBuffer ** buf) { + GstOMXAcquireBufferReturn ret = GST_OMX_ACQUIRE_BUFFER_ERROR; GstOMXComponent *comp; OMX_ERRORTYPE err; - GstOMXBuffer *buf = NULL; + GstOMXBuffer *_buf = NULL; - g_return_val_if_fail (port != NULL, NULL); + g_return_val_if_fail (port != NULL, GST_OMX_ACQUIRE_BUFFER_ERROR); + g_return_val_if_fail (buf != NULL, GST_OMX_ACQUIRE_BUFFER_ERROR); + + *buf = NULL; comp = port->comp; GST_DEBUG_OBJECT (comp->parent, "Acquiring buffer from port %u", port->index); g_mutex_lock (port->port_lock); - if (port->flushing) - goto done; /* Check if the component is in an error state */ if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { GST_ERROR_OBJECT (comp->parent, "Component is in error state: %d", err); + ret = GST_OMX_ACQUIRE_BUFFER_ERROR; + goto done; + } + + /* Check if the port is flushing */ + if (port->flushing) { + ret = GST_OMX_ACQUIRE_BUFFER_FLUSHING; + goto done; + } + + /* Check if the port needs to be reconfigured */ + if (port->settings_changed) { + GST_DEBUG_OBJECT (comp->parent, "Settings changed for port %u", + port->index); + ret = GST_OMX_ACQUIRE_BUFFER_RECONFIGURE; goto done; } @@ -717,18 +734,43 @@ gst_omx_port_acquire_buffer (GstOMXPort * port) /* Check if the component is in an error state */ if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { GST_ERROR_OBJECT (comp->parent, "Component is in error state: %d", err); + ret = GST_OMX_ACQUIRE_BUFFER_ERROR; goto done; } - buf = g_queue_pop_head (port->pending_buffers); + /* Check if the port is flushing */ + if (port->flushing) { + ret = GST_OMX_ACQUIRE_BUFFER_FLUSHING; + goto done; + } + + /* Check if the port needs to be reconfigured */ + /* FIXME: There might still be pending buffers for the + * previous configuration */ + if (port->settings_changed) { + GST_DEBUG_OBJECT (comp->parent, "Settings changed for port %u", + port->index); + ret = GST_OMX_ACQUIRE_BUFFER_RECONFIGURE; + goto done; + } + + _buf = g_queue_pop_head (port->pending_buffers); + + if (_buf) { + ret = GST_OMX_ACQUIRE_BUFFER_OK; + *buf = _buf; + } else { + GST_ERROR_OBJECT (comp->parent, "Unexpectactly got no buffer"); + ret = GST_OMX_ACQUIRE_BUFFER_ERROR; + } done: g_mutex_unlock (port->port_lock); - GST_DEBUG_OBJECT (comp->parent, "Acquired buffer %p from port %u", buf, - port->index); + GST_DEBUG_OBJECT (comp->parent, "Acquired buffer %p from port %u: %d", buf, + port->index, ret); - return buf; + return ret; } OMX_ERRORTYPE diff --git a/omx/gstomx.h b/omx/gstomx.h index f5f03413b8..646d791102 100644 --- a/omx/gstomx.h +++ b/omx/gstomx.h @@ -33,6 +33,13 @@ typedef enum _GstOMXPortDirection GstOMXPortDirection; typedef struct _GstOMXComponent GstOMXComponent; typedef struct _GstOMXBuffer GstOMXBuffer; +typedef enum { + GST_OMX_ACQUIRE_BUFFER_OK = 0, + GST_OMX_ACQUIRE_BUFFER_FLUSHING, + GST_OMX_ACQUIRE_BUFFER_RECONFIGURE, + GST_OMX_ACQUIRE_BUFFER_ERROR +} GstOMXAcquireBufferReturn; + struct _GstOMXCore { /* Handle to the OpenMAX IL core shared library */ GModule *module; @@ -132,7 +139,7 @@ GstOMXPort * gst_omx_component_get_port (GstOMXComponent * comp, guint32 in void gst_omx_port_get_port_definition (GstOMXPort * port, OMX_PARAM_PORTDEFINITIONTYPE * port_def); gboolean gst_omx_port_update_port_definition (GstOMXPort *port, OMX_PARAM_PORTDEFINITIONTYPE *port_definition); -GstOMXBuffer * gst_omx_port_acquire_buffer (GstOMXPort *port); +GstOMXAcquireBufferReturn gst_omx_port_acquire_buffer (GstOMXPort *port, GstOMXBuffer **buf); OMX_ERRORTYPE gst_omx_port_release_buffer (GstOMXPort *port, GstOMXBuffer *buf); OMX_ERRORTYPE gst_omx_port_set_flushing (GstOMXPort *port, gboolean flush); diff --git a/omx/gstomxvideodec.c b/omx/gstomxvideodec.c index 2f435bb67e..22dd561efb 100644 --- a/omx/gstomxvideodec.c +++ b/omx/gstomxvideodec.c @@ -303,18 +303,17 @@ gst_omx_video_dec_loop (GstOMXVideoDec * self) GstOMXBuffer *buf = NULL; GstVideoFrame *frame; GstFlowReturn flow_ret = GST_FLOW_OK; + GstOMXAcquireBufferReturn acq_return; - buf = gst_omx_port_acquire_buffer (port); - if (!buf) { - if (gst_omx_component_get_last_error (self->component) != OMX_ErrorNone) { - goto component_error; - } else if (!gst_omx_port_is_settings_changed (self->out_port)) { - goto flushing; - } + acq_return = gst_omx_port_acquire_buffer (port, &buf); + if (acq_return == GST_OMX_ACQUIRE_BUFFER_ERROR) { + goto component_error; + } else if (acq_return == GST_OMX_ACQUIRE_BUFFER_FLUSHING) { + goto flushing; } if (!GST_PAD_CAPS (GST_BASE_VIDEO_CODEC_SRC_PAD (self)) - || gst_omx_port_is_settings_changed (self->out_port)) { + || acq_return == GST_OMX_ACQUIRE_BUFFER_RECONFIGURE) { GstVideoState *state = &GST_BASE_VIDEO_CODEC (self)->state; OMX_PARAM_PORTDEFINITIONTYPE port_def; @@ -337,16 +336,15 @@ gst_omx_video_dec_loop (GstOMXVideoDec * self) /* gst_util_double_to_fraction (port_def.format.video.xFramerate / ((gdouble) 0xffff), &state->fps_n, &state->fps_d); */ gst_base_video_decoder_set_src_caps (GST_BASE_VIDEO_DECODER (self)); - if (gst_omx_port_is_settings_changed (self->out_port)) { + if (acq_return == GST_OMX_ACQUIRE_BUFFER_RECONFIGURE) { if (gst_omx_port_reconfigure (self->out_port) != OMX_ErrorNone) goto reconfigure_error; - } - - /* Get a new buffer */ - if (!buf) return; + } } + g_assert (acq_return == GST_OMX_ACQUIRE_BUFFER_OK && buf != NULL); + GST_DEBUG_OBJECT (self, "Handling buffer: 0x%08x %lu", buf->omx_buf->nFlags, buf->omx_buf->nTimeStamp); @@ -635,126 +633,105 @@ static GstFlowReturn gst_omx_video_dec_handle_frame (GstBaseVideoDecoder * decoder, GstVideoFrame * frame) { + GstOMXAcquireBufferReturn acq_ret = GST_OMX_ACQUIRE_BUFFER_ERROR; GstOMXVideoDec *self; GstOMXBuffer *buf; GstBuffer *codec_data = NULL; + guint offset = 0; + GstClockTime timestamp, duration, timestamp_offset = 0; self = GST_OMX_VIDEO_DEC (decoder); GST_DEBUG_OBJECT (self, "Handling frame"); - if (gst_omx_port_is_flushing (self->in_port)) - goto flushing; - if (gst_omx_component_get_last_error (self->component) != OMX_ErrorNone) - goto component_error; + GST_OBJECT_LOCK (self); + self->pending_frames = g_list_append (self->pending_frames, frame); + GST_OBJECT_UNLOCK (self); - if (gst_omx_port_is_settings_changed (self->in_port)) { - if (gst_omx_port_reconfigure (self->in_port) != OMX_ErrorNone) - goto reconfigure_error; - } + timestamp = frame->presentation_timestamp; + duration = frame->presentation_duration; - if (self->codec_data) { - codec_data = self->codec_data; + while (offset < GST_BUFFER_SIZE (frame->sink_buffer)) { + acq_ret = gst_omx_port_acquire_buffer (self->in_port, &buf); - retry_codec_data: - buf = gst_omx_port_acquire_buffer (self->in_port); - if (!buf) { - if (gst_omx_component_get_last_error (self->component) != OMX_ErrorNone) { - goto component_error; - } else if (gst_omx_port_is_settings_changed (self->in_port)) { - if (gst_omx_port_reconfigure (self->in_port) != OMX_ErrorNone) - goto reconfigure_error; - goto retry_codec_data; - } else { - goto flushing; - } + if (acq_ret == GST_OMX_ACQUIRE_BUFFER_ERROR) { + goto component_error; + } else if (acq_ret == GST_OMX_ACQUIRE_BUFFER_FLUSHING) { + goto flushing; + } else if (acq_ret == GST_OMX_ACQUIRE_BUFFER_RECONFIGURE) { + if (gst_omx_port_reconfigure (self->in_port) != OMX_ErrorNone) + goto reconfigure_error; + /* Now get a new buffer and fill it */ + continue; } - if (buf->omx_buf->nAllocLen < GST_BUFFER_SIZE (codec_data)) { - gst_omx_port_release_buffer (self->in_port, buf); - goto too_large_codec_data; - } + g_assert (acq_ret == GST_OMX_ACQUIRE_BUFFER_OK && buf != NULL); - buf->omx_buf->nFlags |= OMX_BUFFERFLAG_CODECCONFIG; - buf->omx_buf->nFilledLen = GST_BUFFER_SIZE (codec_data); - memcpy (buf->omx_buf->pBuffer + buf->omx_buf->nOffset, - GST_BUFFER_DATA (codec_data), GST_BUFFER_SIZE (codec_data)); + if (self->codec_data) { + codec_data = self->codec_data; - gst_omx_port_release_buffer (self->in_port, buf); - gst_buffer_replace (&self->codec_data, NULL); - } - - { - guint offset = 0; - GstClockTime timestamp, duration, timestamp_offset = 0; - - GST_OBJECT_LOCK (self); - self->pending_frames = g_list_append (self->pending_frames, frame); - GST_OBJECT_UNLOCK (self); - - timestamp = frame->presentation_timestamp; - duration = frame->presentation_duration; - - while (offset < GST_BUFFER_SIZE (frame->sink_buffer)) { - buf = gst_omx_port_acquire_buffer (self->in_port); - - if (!buf) { - if (gst_omx_component_get_last_error (self->component) != OMX_ErrorNone) { - goto component_error; - } else if (gst_omx_port_is_settings_changed (self->in_port)) { - if (gst_omx_port_reconfigure (self->in_port) != OMX_ErrorNone) - goto reconfigure_error; - continue; - } else { - goto flushing; - } + if (buf->omx_buf->nAllocLen < GST_BUFFER_SIZE (codec_data)) { + gst_omx_port_release_buffer (self->in_port, buf); + goto too_large_codec_data; } - /* Copy the buffer content in chunks of size as requested - * by the port */ - buf->omx_buf->nFilledLen = - MIN (GST_BUFFER_SIZE (frame->sink_buffer) - offset, - buf->omx_buf->nAllocLen - buf->omx_buf->nOffset); + buf->omx_buf->nFlags |= OMX_BUFFERFLAG_CODECCONFIG; + buf->omx_buf->nFilledLen = GST_BUFFER_SIZE (codec_data); memcpy (buf->omx_buf->pBuffer + buf->omx_buf->nOffset, - GST_BUFFER_DATA (frame->sink_buffer) + offset, - buf->omx_buf->nFilledLen); + GST_BUFFER_DATA (codec_data), GST_BUFFER_SIZE (codec_data)); - /* Interpolate timestamps if we're passing the buffer - * in multiple chunks */ - if (offset != 0 && duration != GST_CLOCK_TIME_NONE) { - timestamp_offset = - gst_util_uint64_scale (offset, duration, - GST_BUFFER_SIZE (frame->sink_buffer)); - } - - if (timestamp != GST_CLOCK_TIME_NONE) { - buf->omx_buf->nTimeStamp = - gst_util_uint64_scale (timestamp + timestamp_offset, - OMX_TICKS_PER_SECOND, GST_SECOND); - } - if (duration != GST_CLOCK_TIME_NONE) { - buf->omx_buf->nTickCount = - gst_util_uint64_scale (buf->omx_buf->nFilledLen, duration, - GST_BUFFER_SIZE (frame->sink_buffer)); - } - - if (offset == 0) { - BufferIdentification *id = g_slice_new0 (BufferIdentification); - - id->timestamp = buf->omx_buf->nTimeStamp; - frame->coder_hook = id; - } - - /* TODO: Set flags - * - OMX_BUFFERFLAG_DECODEONLY for buffers that are outside - * the segment - * - OMX_BUFFERFLAG_SYNCFRAME for non-delta frames - * - OMX_BUFFERFLAG_ENDOFFRAME for parsed input - */ - - offset += buf->omx_buf->nFilledLen; gst_omx_port_release_buffer (self->in_port, buf); + gst_buffer_replace (&self->codec_data, NULL); + /* Acquire new buffer for the actual frame */ + continue; } + + /* Now handle the frame */ + + /* Copy the buffer content in chunks of size as requested + * by the port */ + buf->omx_buf->nFilledLen = + MIN (GST_BUFFER_SIZE (frame->sink_buffer) - offset, + buf->omx_buf->nAllocLen - buf->omx_buf->nOffset); + memcpy (buf->omx_buf->pBuffer + buf->omx_buf->nOffset, + GST_BUFFER_DATA (frame->sink_buffer) + offset, + buf->omx_buf->nFilledLen); + + /* Interpolate timestamps if we're passing the buffer + * in multiple chunks */ + if (offset != 0 && duration != GST_CLOCK_TIME_NONE) { + timestamp_offset = + gst_util_uint64_scale (offset, duration, + GST_BUFFER_SIZE (frame->sink_buffer)); + } + + if (timestamp != GST_CLOCK_TIME_NONE) { + buf->omx_buf->nTimeStamp = + gst_util_uint64_scale (timestamp + timestamp_offset, + OMX_TICKS_PER_SECOND, GST_SECOND); + } + if (duration != GST_CLOCK_TIME_NONE) { + buf->omx_buf->nTickCount = + gst_util_uint64_scale (buf->omx_buf->nFilledLen, duration, + GST_BUFFER_SIZE (frame->sink_buffer)); + } + + if (offset == 0) { + BufferIdentification *id = g_slice_new0 (BufferIdentification); + + id->timestamp = buf->omx_buf->nTimeStamp; + frame->coder_hook = id; + } + + /* TODO: Set flags + * - OMX_BUFFERFLAG_DECODEONLY for buffers that are outside + * the segment + * - OMX_BUFFERFLAG_SYNCFRAME for non-delta frames + * - OMX_BUFFERFLAG_ENDOFFRAME for parsed input + */ + + offset += buf->omx_buf->nFilledLen; + gst_omx_port_release_buffer (self->in_port, buf); } return GST_FLOW_OK; @@ -793,6 +770,7 @@ gst_omx_video_dec_finish (GstBaseVideoDecoder * decoder) { GstOMXVideoDec *self; GstOMXBuffer *buf; + GstOMXAcquireBufferReturn acq_ret; self = GST_OMX_VIDEO_DEC (decoder); @@ -801,8 +779,8 @@ gst_omx_video_dec_finish (GstBaseVideoDecoder * decoder) /* Send an EOS buffer to the component and let the base * class drop the EOS event. We will send it later when * the EOS buffer arrives on the output port. */ - buf = gst_omx_port_acquire_buffer (self->in_port); - if (buf) { + acq_ret = gst_omx_port_acquire_buffer (self->in_port, &buf); + if (acq_ret == GST_OMX_ACQUIRE_BUFFER_OK) { buf->omx_buf->nFlags |= OMX_BUFFERFLAG_EOS; gst_omx_port_release_buffer (self->in_port, buf); }