[Mesa-dev] [PATCH 4/4] st/omx: initial OpenMAX H264 encoder v6
Emil Velikov
emil.l.velikov at gmail.com
Tue Feb 11 10:56:02 PST 2014
On 11/02/14 17:27, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> v2 (chk): fix eos handling
> v3 (leo): implement scaling configuration support
> v4 (leo): fix bitrate bug
> v5 (chk): add workaround for bug in Bellagio
> v6 (chk): fix div by 0 if framerate isn't known,
> user separate pipe object for scale and transfer,
> always flush the transfer pipe before encoding
>
Hi Christian
Not familiar with the API, so a rather naive question:
* If vid_enc_Constructor fails I'm assuming that vid_enc_Destructor
will be executed to clean-up the mess.
* The following four functions have some massive switch statements,
possibly consider
- Adding a idx to variable type map
- Move checkHeader(param, map[idx]) before the switch
- Finally drop the brakets in the case statements
vid_enc_SetParameter
vid_enc_GetParameter
vid_enc_SetConfig
vid_enc_GetConfig
> Signed-off-by: Christian König <christian.koenig at amd.com>
> Signed-off-by: Leo Liu <leo.liu at amd.com>
> ---
> src/gallium/drivers/radeon/radeon_vce_40_2_2.c | 2 +-
> src/gallium/state_trackers/omx/Makefile.am | 3 +-
> src/gallium/state_trackers/omx/entrypoint.c | 9 +-
> src/gallium/state_trackers/omx/vid_dec.c | 15 +-
> src/gallium/state_trackers/omx/vid_enc.c | 853 +++++++++++++++++++++++++
> src/gallium/state_trackers/omx/vid_enc.h | 81 +++
> 6 files changed, 954 insertions(+), 9 deletions(-)
> create mode 100644 src/gallium/state_trackers/omx/vid_enc.c
> create mode 100644 src/gallium/state_trackers/omx/vid_enc.h
>
[snip]
> +OMX_ERRORTYPE vid_enc_LoaderComponent(stLoaderComponentType *comp)
> +{
Better positioned goto statements will remove all of the conditionals in
the error path.
if (variable == NULL) and be rewritten as if (!variable), to be
consistent with the rest of the code.
> + /* component 1 - video encoder */
> + comp->componentVersion.s.nVersionMajor = 0;
> + comp->componentVersion.s.nVersionMinor = 0;
> + comp->componentVersion.s.nRevision = 0;
> + comp->componentVersion.s.nStep = 1;
> + comp->name_specific_length = 1;
> +
> + comp->name = CALLOC(1, OMX_MAX_STRINGNAME_SIZE);
> + if (comp->name == NULL)
> + goto error;
> +
> + comp->name_specific = CALLOC(comp->name_specific_length, sizeof(char *));
> + if (comp->name_specific == NULL)
> + goto error;
> +
> + comp->name_specific[0] = CALLOC(1, OMX_MAX_STRINGNAME_SIZE);
> + if (comp->name_specific[0] == NULL)
> + goto error;
> +
> + comp->role_specific = CALLOC(comp->name_specific_length, sizeof(char *));
> + if (comp->role_specific == NULL)
> + goto error;
> +
> + comp->role_specific[0] = CALLOC(1, OMX_MAX_STRINGNAME_SIZE);
> + if (comp->role_specific[0] == NULL)
> + goto error;
> +
> + vid_enc_name(comp->name);
> + vid_enc_name_avc(comp->name_specific[0]);
> +
> + strcpy(comp->role_specific[0], OMX_VID_ENC_AVC_ROLE);
> +
> + comp->constructor = vid_enc_Constructor;
> +
> + return 2;
> +
Why do we return 2, as there is only one component (ie the encoder) ?
> +error:
> + FREE(comp->name);
> +
> + if (comp->name_specific) {
> + FREE(comp->name_specific[0]);
> + FREE(comp->name_specific);
> + }
> +
> + if (comp->role_specific) {
> + FREE(comp->role_specific[0]);
> + FREE(comp->role_specific);
> + }
> +
> + return OMX_ErrorInsufficientResources;
> +}
> +
> +static OMX_ERRORTYPE vid_enc_Constructor(OMX_COMPONENTTYPE *comp, OMX_STRING name)
> +{
> + vid_enc_PrivateType *priv;
> + omx_base_video_PortType *port;
> + struct pipe_screen *screen;
> + OMX_ERRORTYPE r;
> + int i;
> +
> + assert(!comp->pComponentPrivate);
> +
> + priv = comp->pComponentPrivate = CALLOC(1, sizeof(vid_enc_PrivateType));
> + if (!priv)
> + return OMX_ErrorInsufficientResources;
> +
> + r = omx_base_filter_Constructor(comp, name);
> + if (r)
> + return r;
> +
> + priv->BufferMgmtCallback = vid_enc_BufferEncoded;
> + priv->messageHandler = vid_enc_MessageHandler;
> + priv->destructor = vid_enc_Destructor;
> +
> + comp->SetParameter = vid_enc_SetParameter;
> + comp->GetParameter = vid_enc_GetParameter;
> + comp->GetConfig = vid_enc_GetConfig;
> + comp->SetConfig = vid_enc_SetConfig;
> +
> + priv->screen = omx_get_screen();
> + if (!priv->screen)
> + return OMX_ErrorInsufficientResources;
> +
> + screen = priv->screen->pscreen;
> + priv->s_pipe = screen->context_create(screen, priv->screen);
> + if (!priv->s_pipe)
> + return OMX_ErrorInsufficientResources;
> +
> + priv->t_pipe = screen->context_create(screen, priv->screen);
> + if (!priv->t_pipe)
> + return OMX_ErrorInsufficientResources;
> +
> + if (!vl_compositor_init(&priv->compositor, priv->s_pipe))
> + return OMX_ErrorInsufficientResources;
> +
> + if (!vl_compositor_init_state(&priv->cstate, priv->s_pipe))
> + return OMX_ErrorInsufficientResources;
> +
> + priv->sPortTypesParam[OMX_PortDomainVideo].nStartPortNumber = 0;
> + priv->sPortTypesParam[OMX_PortDomainVideo].nPorts = 2;
> + priv->ports = CALLOC(2, sizeof(omx_base_PortType *));
> + if (!priv->ports)
> + return OMX_ErrorInsufficientResources;
> +
> + for (i = 0; i < 2; ++i) {
Is is possible for a encoder to have more than two number of ports ?
Would it be worth adding a define ?
> + priv->ports[i] = CALLOC(1, sizeof(omx_base_video_PortType));
> + if (!priv->ports[i])
> + return OMX_ErrorInsufficientResources;
> +
> + base_video_port_Constructor(comp, &priv->ports[i], i, i == 0);
> + }
> +
> + port = (omx_base_video_PortType *)priv->ports[OMX_BASE_FILTER_INPUTPORT_INDEX];
> + port->sPortParam.format.video.nFrameWidth = 176;
> + port->sPortParam.format.video.nFrameHeight = 144;
> + port->sPortParam.format.video.eColorFormat = OMX_COLOR_FormatYUV420SemiPlanar;
> + port->sVideoParam.eColorFormat = OMX_COLOR_FormatYUV420SemiPlanar;
> + port->sPortParam.nBufferCountActual = 8;
> + port->sPortParam.nBufferCountMin = 4;
> +
> + port->Port_SendBufferFunction = vid_enc_EncodeFrame;
> + port->Port_AllocateBuffer = vid_enc_AllocateInBuffer;
> + port->Port_UseBuffer = vid_enc_UseInBuffer;
> + port->Port_FreeBuffer = vid_enc_FreeInBuffer;
> +
> + port = (omx_base_video_PortType *)priv->ports[OMX_BASE_FILTER_OUTPUTPORT_INDEX];
> + strcpy(port->sPortParam.format.video.cMIMEType,"video/H264");
> + port->sPortParam.format.video.nFrameWidth = 176;
> + port->sPortParam.format.video.nFrameHeight = 144;
> + port->sPortParam.format.video.eCompressionFormat = OMX_VIDEO_CodingAVC;
> + port->sVideoParam.eCompressionFormat = OMX_VIDEO_CodingAVC;
> +
> + port->Port_AllocateBuffer = vid_enc_AllocateOutBuffer;
> + port->Port_FreeBuffer = vid_enc_FreeOutBuffer;
> +
> + priv->bitrate.eControlRate = OMX_Video_ControlRateDisable;
> + priv->bitrate.nTargetBitrate = 0;
> +
> + priv->quant.nQpI = OMX_VID_ENC_QUANT_I_FRAMES_DEFAULT;
> + priv->quant.nQpP = OMX_VID_ENC_QUANT_P_FRAMES_DEFAULT;
> + priv->quant.nQpB = OMX_VID_ENC_QUANT_B_FRAMES_DEFAULT;
> +
> + priv->force_pic_type.IntraRefreshVOP = OMX_FALSE;
> + priv->frame_num = 0;
> +
> + priv->scale.xWidth = OMX_VID_ENC_SCALING_WIDTH_DEFAULT;
> + priv->scale.xHeight = OMX_VID_ENC_SCALING_WIDTH_DEFAULT;
> +
> + return OMX_ErrorNone;
> +}
> +
[snip]
> +static OMX_ERRORTYPE vid_enc_AllocateOutBuffer(omx_base_PortType *port, OMX_INOUT OMX_BUFFERHEADERTYPE **buf,
> + OMX_IN OMX_U32 idx, OMX_IN OMX_PTR private, OMX_IN OMX_U32 size)
> +{
> + OMX_ERRORTYPE r;
> +
> + r = base_port_AllocateBuffer(port, buf, idx, private, size);
> + if (r)
> + return r;
> +
> + FREE((*buf)->pBuffer);
> + (*buf)->pBuffer = NULL;
> + (*buf)->pOutputPortPrivate = CALLOC(1, sizeof(struct output_buf_private));
> +
I know I'm rather pedantic but can we handle CALLOC failure, please.
> + return OMX_ErrorNone;
> +}
> +
> +static OMX_ERRORTYPE vid_enc_FreeOutBuffer(omx_base_PortType *port, OMX_U32 idx, OMX_BUFFERHEADERTYPE *buf)
> +{
> + OMX_COMPONENTTYPE* comp = port->standCompContainer;
> + vid_enc_PrivateType *priv = comp->pComponentPrivate;
> +
> + if (buf->pOutputPortPrivate) {
> + struct output_buf_private *outp = buf->pOutputPortPrivate;
> + if (outp->transfer)
> + pipe_transfer_unmap(priv->t_pipe, outp->transfer);
> + pipe_resource_reference(&outp->bitstream, NULL);
> + FREE(outp);
> + buf->pOutputPortPrivate = NULL;
> + }
> + buf->pBuffer = NULL;
> +
> + return base_port_FreeBuffer(port, idx, buf);
> +}
> +
> +static OMX_ERRORTYPE vid_enc_EncodeFrame(omx_base_PortType *port, OMX_BUFFERHEADERTYPE *buf)
> +{
> + OMX_COMPONENTTYPE* comp = port->standCompContainer;
> + vid_enc_PrivateType *priv = comp->pComponentPrivate;
> + struct input_buf_private *inp = buf->pInputPortPrivate;
> + unsigned size = priv->ports[OMX_BASE_FILTER_OUTPUTPORT_INDEX]->sPortParam.nBufferSize;
> + OMX_VIDEO_PORTDEFINITIONTYPE *def = &port->sPortParam.format.video;
> + struct pipe_h264_enc_picture_desc picture;
> + struct pipe_video_buffer *vbuf = inp->buf;
> +
> + pipe_resource_reference(&inp->bitstream, NULL);
> +
> + if (buf->nFilledLen == 0) {
> + if (buf->nFlags & OMX_BUFFERFLAG_EOS)
> + buf->nFilledLen = buf->nAllocLen;
> + return base_port_SendBufferFunction(port, buf);
> + }
> +
> + if (buf->pOutputPortPrivate) {
> + vbuf = buf->pOutputPortPrivate;
> + } else {
> + /* ------- load input image into video buffer ---- */
> + struct pipe_sampler_view **views;
> + struct pipe_box box = {};
> + void *ptr;
> +
> + views = inp->buf->get_sampler_view_planes(vbuf);
> + if (!views)
> + return OMX_ErrorInsufficientResources;
> +
> + ptr = buf->pBuffer;
> +
> + box.width = def->nFrameWidth;
> + box.height = def->nFrameHeight;
> + box.depth = 1;
> +
> + priv->s_pipe->transfer_inline_write(priv->s_pipe, views[0]->texture, 0,
> + PIPE_TRANSFER_WRITE, &box,
> + ptr, def->nStride, 0);
> +
> +
> + ptr = ((uint8_t*)buf->pBuffer) + (def->nStride * box.height);
> +
> + box.width = def->nFrameWidth / 2;
> + box.height = def->nFrameHeight / 2;
> + box.depth = 1;
> +
> + priv->s_pipe->transfer_inline_write(priv->s_pipe, views[1]->texture, 0,
> + PIPE_TRANSFER_WRITE, &box,
> + ptr, def->nStride, 0);
> + }
> +
> + /* -------------- scale input image --------- */
> +
> + if (priv->scale_buffer) {
> + struct vl_compositor *compositor = &priv->compositor;
> + struct vl_compositor_state *s = &priv->cstate;
> + struct pipe_sampler_view **views;
> + struct pipe_surface **dst_surface;
> + unsigned i;
> +
> + views = vbuf->get_sampler_view_planes(vbuf);
> + dst_surface = priv->scale_buffer->get_surfaces(priv->scale_buffer);
> + vl_compositor_clear_layers(s);
> + for (i = 0; i < VL_MAX_SURFACES; ++i) {
> + if (!views[i] || !dst_surface[i])
> + continue;
> + vl_compositor_set_rgba_layer(s, compositor, 0, views[i], NULL, NULL, NULL);
> + vl_compositor_render(s, compositor, dst_surface[i], NULL, false);
> + }
> +
> + size = priv->scale.xWidth * priv->scale.xHeight * 2;
> + vbuf = priv->scale_buffer;
> + }
> +
> + priv->s_pipe->flush(priv->s_pipe, NULL, 0);
> +
> + /* -------------- allocate output buffer --------- */
> +
> + inp->bitstream = pipe_buffer_create(priv->s_pipe->screen, PIPE_BIND_VERTEX_BUFFER,
> + PIPE_USAGE_STREAM, size);
> +
> + /* -------------- decode frame --------- */
> +
> + switch (priv->bitrate.eControlRate) {
> + case OMX_Video_ControlRateVariable:
> + picture.rate_ctrl.rate_ctrl_method = PIPE_H264_ENC_RATE_CONTROL_METHOD_VARIABLE;
> + break;
> + case OMX_Video_ControlRateConstant:
> + picture.rate_ctrl.rate_ctrl_method = PIPE_H264_ENC_RATE_CONTROL_METHOD_CONSTANT;
> + break;
> + case OMX_Video_ControlRateVariableSkipFrames:
> + picture.rate_ctrl.rate_ctrl_method = PIPE_H264_ENC_RATE_CONTROL_METHOD_VARIABLE_SKIP;
> + break;
> + case OMX_Video_ControlRateConstantSkipFrames:
> + picture.rate_ctrl.rate_ctrl_method = PIPE_H264_ENC_RATE_CONTROL_METHOD_CONSTANT_SKIP;
> + break;
> + default:
> + picture.rate_ctrl.rate_ctrl_method = PIPE_H264_ENC_RATE_CONTROL_METHOD_DISABLE;
> + break;
> + }
> +
> + if (picture.rate_ctrl.rate_ctrl_method != PIPE_H264_ENC_RATE_CONTROL_METHOD_DISABLE) {
> + if (priv->bitrate.nTargetBitrate < OMX_VID_ENC_BITRATE_MIN)
> + picture.rate_ctrl.target_bitrate = OMX_VID_ENC_BITRATE_MIN;
> + else if (priv->bitrate.nTargetBitrate < OMX_VID_ENC_BITRATE_MAX)
> + picture.rate_ctrl.target_bitrate = priv->bitrate.nTargetBitrate;
> + else
> + picture.rate_ctrl.target_bitrate = OMX_VID_ENC_BITRATE_MAX;
> + picture.rate_ctrl.peak_bitrate = picture.rate_ctrl.target_bitrate;
> + picture.rate_ctrl.frame_rate_den = OMX_VID_ENC_CONTROL_FRAME_RATE_DEN_DEFAULT;
> + picture.rate_ctrl.frame_rate_num = ((priv->frame_rate) >> 16) * picture.rate_ctrl.frame_rate_den;
> + if (picture.rate_ctrl.target_bitrate < OMX_VID_ENC_BITRATE_MEDIAN)
> + picture.rate_ctrl.vbv_buffer_size = MIN2((picture.rate_ctrl.target_bitrate * 2.75), OMX_VID_ENC_BITRATE_MEDIAN);
> + else
> + picture.rate_ctrl.vbv_buffer_size = picture.rate_ctrl.target_bitrate;
> +
> + if (picture.rate_ctrl.frame_rate_num) {
> + unsigned long long t = picture.rate_ctrl.target_bitrate;
> + t *= picture.rate_ctrl.frame_rate_den;
> + picture.rate_ctrl.target_bits_picture = t / picture.rate_ctrl.frame_rate_num;
> + } else {
> + picture.rate_ctrl.target_bits_picture = picture.rate_ctrl.target_bitrate;
> + }
> + picture.rate_ctrl.peak_bits_picture_integer = picture.rate_ctrl.target_bits_picture;
> + picture.rate_ctrl.peak_bits_picture_fraction = 0;
Use local variable to store picture.rate_ctrl, to ease the eyes ?
Cheers
-Emil
More information about the mesa-dev
mailing list