[v2,1/3] libavcodec: Add H264/MPEG4 encoders based on OpenMAX IL

Message ID 1460029429-26222-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö April 7, 2016, 11:43 a.m.
---
Fixed most of wm4's comments, except for the static pixfmt init
and getting rid of dlopen.
---
 configure              |   6 +
 libavcodec/Makefile    |   2 +
 libavcodec/allcodecs.c |   2 +
 libavcodec/omx.c       | 783 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 793 insertions(+)
 create mode 100644 libavcodec/omx.c

Comments

Diego Biurrun April 7, 2016, 1:42 p.m. | #1
On Thu, Apr 07, 2016 at 02:43:47PM +0300, Martin Storsjö wrote:
> ---
>  configure              |   6 +
>  libavcodec/Makefile    |   2 +
>  libavcodec/allcodecs.c |   2 +
>  libavcodec/omx.c       | 783
>  +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 793 insertions(+)
>  create mode 100644 libavcodec/omx.c

version bump, docs, changelog

> --- a/configure
> +++ b/configure
> @@ -1878,6 +1880,7 @@ mpegvideo_select="blockdsp hpeldsp idctdsp me_cmp mpeg_er videodsp"
>  nvenc_extralibs='$ldl'
> +omx_extralibs='$ldl'
>  qsvdec_select="qsv"
> @@ -1954,6 +1957,7 @@ h263p_encoder_select="h263_encoder"
>  h264_nvenc_encoder_deps="nvenc"
> +h264_omx_encoder_deps="omx dlopen pthreads"
>  h264_qsv_decoder_deps="libmfx"
> @@ -2019,6 +2023,7 @@ mpeg2_qsv_encoder_deps="libmfx"
>  mpeg4_encoder_select="h263_encoder"
> +mpeg4_omx_encoder_deps="omx dlopen pthreads"
>  msa1_decoder_select="mss34dsp"

Having omx depend on dlopen and pthreads would be cleaner IMO.

> --- /dev/null
> +++ b/libavcodec/omx.c
> @@ -0,0 +1,783 @@
> +
> +static OMX_TICKS to_omx_ticks(int64_t value) {

{ on the next line

> +    OMX_TICKS s;
> +    s.nLowPart = value;
> +    s.nHighPart = value >> 32;

align

> +static int64_t from_omx_ticks(OMX_TICKS value) {

{ on the next line

> +#define INIT_STRUCT(x) do {                                               \
> +        x.nSize = sizeof(x);                                              \
> +        x.nVersion = s->version;                                          \

align

> +static av_cold void *dlsym2(void *handle, const char *symbol, const char *prefix)

2?

> +static av_cold int omx_init(void *logctx, const char *libname, const char *prefix) {

{ on the next line

> +static av_cold void omx_deinit(void) {

{ on the next line

> +static OMX_ERRORTYPE event_handler(OMX_HANDLETYPE component, OMX_PTR app_data, OMX_EVENTTYPE event, OMX_U32 data1, OMX_U32 data2, OMX_PTR event_data)

nit: long line

> +{
> +    OMXCodecContext *s = app_data;
> +    switch (event) {
> +    case OMX_EventError:
> +        pthread_mutex_lock(&s->state_mutex);
> +        av_log(s->avctx, AV_LOG_ERROR, "OMX error %x\n", (int) data1);

Maybe use Posix integer printf specifiers instead of casting?
In any case unsigned looks like a better choice than signed int; I
assume that OMX_U32 is an unsigned type.

> +static OMX_ERRORTYPE fill_buffer_done(OMX_HANDLETYPE component, OMX_PTR app_data, OMX_BUFFERHEADERTYPE *buffer)

nit: long line

> +    in_port_params.bEnabled = OMX_TRUE;
> +    in_port_params.bPopulated = OMX_FALSE;
> +    in_port_params.eDomain = OMX_PortDomainVideo;
> +    s->num_in_buffers = in_port_params.nBufferCountActual;
> +
> +    in_port_params.format.video.pNativeRender = NULL;
> +    in_port_params.format.video.nStride = -1;
> +    in_port_params.format.video.nSliceHeight = -1;
> +    in_port_params.format.video.xFramerate = 30 << 16;
> +    in_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
> +    in_port_params.format.video.eColorFormat = s->color_format;
> +    s->stride = avctx->width;
> +    s->plane_size = avctx->height;
> +    in_port_params.format.video.nStride = s->stride;
> +    in_port_params.format.video.nSliceHeight = s->plane_size;

You could align = here and in many other places.

> +    s->in_buffer_headers  = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
> +    s->free_in_buffers    = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
> +    s->out_buffer_headers = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
> +    s->done_out_buffers   = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
> +    if (!s->in_buffer_headers || !s->free_in_buffers || !s->out_buffer_headers || !s->done_out_buffers)
> +        return AVERROR(ENOMEM);

Why is this not a memleak if some of the mallocs fail?

> +            memmove(&s->free_in_buffers[0], &s->free_in_buffers[1], s->num_free_in_buffers * sizeof(OMX_BUFFERHEADERTYPE*));

maybe sizeof(*var)

> +AVCodec ff_mpeg4_omx_encoder = {
> +    .name             = "mpeg4_omx",
> +    .type             = AVMEDIA_TYPE_VIDEO,
> +    .id               = AV_CODEC_ID_MPEG4,
> +    .priv_data_size   = sizeof(OMXCodecContext),
> +    .init             = omx_encode_init,
> +    .encode2          = omx_encode_frame,
> +    .close            = omx_encode_end,
> +    .pix_fmts         = omx_encoder_pix_fmts,
> +    .long_name        = NULL_IF_CONFIG_SMALL("OpenMAX IL MPEG4 video encoder"),
> +    .capabilities     = AV_CODEC_CAP_DELAY,
> +    .priv_class       = &omx_mpeg4enc_class,
> +};

nit: Group name and long_name together

> +AVCodec ff_h264_omx_encoder = {
> +    .name             = "h264_omx",
> +    .type             = AVMEDIA_TYPE_VIDEO,
> +    .id               = AV_CODEC_ID_H264,
> +    .priv_data_size   = sizeof(OMXCodecContext),
> +    .init             = omx_encode_init,
> +    .encode2          = omx_encode_frame,
> +    .close            = omx_encode_end,
> +    .pix_fmts         = omx_encoder_pix_fmts,
> +    .long_name        = NULL_IF_CONFIG_SMALL("OpenMAX IL H264 video encoder"),
> +    .capabilities     = AV_CODEC_CAP_DELAY,
> +    .priv_class       = &omx_h264enc_class,
> +};

same

Diego
Martin Storsjö April 7, 2016, 7:17 p.m. | #2
On Thu, 7 Apr 2016, Diego Biurrun wrote:

>> +static av_cold void *dlsym2(void *handle, const char *symbol, const char *prefix)
>
> 2?

Appending 2 is our own normal naming scheme for "new and improved version 
of function X" :P

Perhaps dlsym_prefix would be more descriptive, even if it places more 
emphasis than wanted on using a prefix, which is used very seldom.

>> +    s->in_buffer_headers  = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
>> +    s->free_in_buffers    = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
>> +    s->out_buffer_headers = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
>> +    s->done_out_buffers   = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
>> +    if (!s->in_buffer_headers || !s->free_in_buffers || !s->out_buffer_headers || !s->done_out_buffers)
>> +        return AVERROR(ENOMEM);
>
> Why is this not a memleak if some of the mallocs fail?

Because if this function returns an error, the caller (omx_encode_init) 
will goto fail and call cleanup, which cleans up any and all allocations.

// Martin
Mark Thompson April 9, 2016, 6:20 p.m. | #3
On 07/04/16 12:43, Martin Storsjö wrote:
> ...
> +
> +static av_cold int omx_component_init(AVCodecContext *avctx, const char *role)
> +{
> +    OMXCodecContext *s = avctx->priv_data;
> +    OMX_PARAM_COMPONENTROLETYPE role_params = { 0 };
> +    OMX_PORT_PARAM_TYPE video_port_params = { 0 };
> +    OMX_PARAM_PORTDEFINITIONTYPE in_port_params = { 0 }, out_port_params = { 0 };
> +    OMX_VIDEO_PARAM_PORTFORMATTYPE video_port_format = { 0 };
> +    OMX_VIDEO_PARAM_BITRATETYPE vid_param_bitrate = { 0 };
> +    OMX_ERRORTYPE err;
> +    int i, default_size;
> +
> +    s->version.s.nVersionMajor = 1;
> +    s->version.s.nVersionMinor = 1;
> +
> +    err = omx_context->ptr_GetHandle(&s->handle, s->component_name, s, (OMX_CALLBACKTYPE*) &callbacks);
> +    if (err != OMX_ErrorNone) {
> +        av_log(avctx, AV_LOG_ERROR, "OMX_GetHandle(%s) failed: %x\n", s->component_name, err);
> +        return AVERROR_UNKNOWN;
> +    }
> +
> +    // This one crashes the mediaserver on qcom, if used over IOMX
> +    INIT_STRUCT(role_params);
> +    av_strlcpy(role_params.cRole, role, sizeof(role_params.cRole));
> +    // Intentionally ignore errors on this one
> +    OMX_SetParameter(s->handle, OMX_IndexParamStandardComponentRole, &role_params);
> +
> +    INIT_STRUCT(video_port_params);
> +    err = OMX_GetParameter(s->handle, OMX_IndexParamVideoInit, &video_port_params);
> +    CHECK(err);
> +
> +    s->in_port = s->out_port = -1;
> +    for (i = 0; i < video_port_params.nPorts; i++) {
> +        int port = video_port_params.nStartPortNumber + i;
> +        OMX_PARAM_PORTDEFINITIONTYPE port_params = { 0 };
> +        INIT_STRUCT(port_params);
> +        port_params.nPortIndex = port;
> +        err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &port_params);
> +        if (err != OMX_ErrorNone) {
> +            av_log(avctx, AV_LOG_WARNING, "port %d error %x\n", port, err);
> +            break;
> +        }
> +        if (port_params.eDir == OMX_DirInput && s->in_port < 0) {
> +            in_port_params = port_params;
> +            s->in_port = port;
> +        } else if (port_params.eDir == OMX_DirOutput && s->out_port < 0) {
> +            out_port_params = port_params;
> +            s->out_port = port;
> +        }
> +    }
> +    if (s->in_port < 0 || s->out_port < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "No in or out port found (in %d out %d)\n", s->in_port, s->out_port);
> +        return AVERROR_UNKNOWN;
> +    }
> +
> +    INIT_STRUCT(video_port_format);
> +    video_port_format.nIndex = 0;
> +    video_port_format.nPortIndex = s->in_port;
> +    OMX_GetParameter(s->handle, OMX_IndexParamVideoPortFormat, &video_port_format);
> +    s->color_format = video_port_format.eColorFormat;
> +    // We only know the actual color format here, even though we've advertised the
> +    // pixel format (YUV420P) via the encoder's pix_fmts array.
> +    if (s->color_format != OMX_COLOR_FormatYUV420Planar &&
> +        s->color_format != OMX_COLOR_FormatYUV420PackedPlanar) {
> +        av_log(avctx, AV_LOG_ERROR, "Pixel format mismatch (%d isn't supported)\n", s->color_format);
> +        return AVERROR_UNKNOWN;
> +    }
> +
> +    in_port_params.bEnabled = OMX_TRUE;
> +    in_port_params.bPopulated = OMX_FALSE;
> +    in_port_params.eDomain = OMX_PortDomainVideo;
> +    s->num_in_buffers = in_port_params.nBufferCountActual;
> +
> +    in_port_params.format.video.pNativeRender = NULL;
> +    in_port_params.format.video.nStride = -1;
> +    in_port_params.format.video.nSliceHeight = -1;

These two writes are redundant.

> +    in_port_params.format.video.xFramerate = 30 << 16;
> +    in_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
> +    in_port_params.format.video.eColorFormat = s->color_format;
> +    s->stride = avctx->width;
> +    s->plane_size = avctx->height;
> +    in_port_params.format.video.nStride = s->stride;
> +    in_port_params.format.video.nSliceHeight = s->plane_size;
> +    if (avctx->framerate.den > 0 && avctx->framerate.num > 0)
> +        in_port_params.format.video.xFramerate = (1 << 16) * avctx->framerate.num / avctx->framerate.den;
> +    else
> +        in_port_params.format.video.xFramerate = (1 << 16) * avctx->time_base.den / avctx->time_base.num;
> +    in_port_params.format.video.nFrameWidth = avctx->width;
> +    in_port_params.format.video.nFrameHeight = avctx->height;
> +    default_size = av_image_get_buffer_size(avctx->pix_fmt, s->stride, s->plane_size, 1);
> +    if (in_port_params.nBufferSize < default_size)
> +        in_port_params.nBufferSize = default_size;
> +
> +    err = OMX_SetParameter(s->handle, OMX_IndexParamPortDefinition, &in_port_params);
> +    CHECK(err);
> +    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &in_port_params);
> +    CHECK(err);

Aren't in_port_params...nStride/nSliceHeight possibly-updated here, if for example the component wants more alignment/padding than you are trying to give it?

> +
> +    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
> +    out_port_params.bEnabled = OMX_TRUE;
> +    out_port_params.bPopulated = OMX_FALSE;
> +    out_port_params.eDomain = OMX_PortDomainVideo;
> +    out_port_params.format.video.pNativeRender = NULL;
> +    out_port_params.format.video.nFrameWidth = avctx->width;
> +    out_port_params.format.video.nFrameHeight = avctx->height;
> +    out_port_params.format.video.nStride = 0;
> +    out_port_params.format.video.nSliceHeight = 0;
> +    out_port_params.format.video.nBitrate = avctx->bit_rate;
> +    out_port_params.format.video.xFramerate = in_port_params.format.video.xFramerate;
> +    out_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
> +    if (avctx->codec->id == AV_CODEC_ID_MPEG4)
> +        out_port_params.format.video.eCompressionFormat = OMX_VIDEO_CodingMPEG4;
> +    else if (avctx->codec->id == AV_CODEC_ID_H264)
> +        out_port_params.format.video.eCompressionFormat = OMX_VIDEO_CodingAVC;
> +    s->num_out_buffers = out_port_params.nBufferCountActual;
> +
> +    err = OMX_SetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
> +    CHECK(err);
> +    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
> +    CHECK(err);
> +
> +    INIT_STRUCT(vid_param_bitrate);
> +    vid_param_bitrate.nPortIndex = s->out_port;
> +    vid_param_bitrate.eControlRate = OMX_Video_ControlRateVariable;
> +    vid_param_bitrate.nTargetBitrate = avctx->bit_rate;
> +    err = OMX_SetParameter(s->handle, OMX_IndexParamVideoBitrate, &vid_param_bitrate);
> +    if (err != OMX_ErrorNone)
> +        av_log(avctx, AV_LOG_WARNING, "Unable to set video bitrate parameter\n");
> +
> +    if (avctx->codec->id == AV_CODEC_ID_H264) {
> +        OMX_VIDEO_PARAM_AVCTYPE avc = { 0 };
> +        INIT_STRUCT(avc);
> +        avc.nPortIndex = s->out_port;
> +        err = OMX_GetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
> +        CHECK(err);
> +        avc.nBFrames = 0;
> +        avc.nPFrames = avctx->gop_size - 1;
> +        err = OMX_SetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
> +        CHECK(err);
> +    }
> +
> +    err = OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateIdle, NULL);
> +    CHECK(err);
> +
> +    s->in_buffer_headers  = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
> +    s->free_in_buffers    = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
> +    s->out_buffer_headers = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
> +    s->done_out_buffers   = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
> +    if (!s->in_buffer_headers || !s->free_in_buffers || !s->out_buffer_headers || !s->done_out_buffers)
> +        return AVERROR(ENOMEM);
> +    for (i = 0; i < s->num_in_buffers && err == OMX_ErrorNone; i++)
> +        err = OMX_AllocateBuffer(s->handle, &s->in_buffer_headers[i],  s->in_port,  s, in_port_params.nBufferSize);
> +    CHECK(err);
> +    s->num_in_buffers = i;
> +    for (i = 0; i < s->num_out_buffers && err == OMX_ErrorNone; i++)
> +        err = OMX_AllocateBuffer(s->handle, &s->out_buffer_headers[i], s->out_port, s, out_port_params.nBufferSize);
> +    CHECK(err);
> +    s->num_out_buffers = i;
> +
> +    if (wait_for_state(s, OMX_StateIdle) < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Didn't get OMX_StateIdle\n");
> +        return AVERROR_UNKNOWN;
> +    }
> +    err = OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateExecuting, NULL);
> +    CHECK(err);
> +    if (wait_for_state(s, OMX_StateExecuting) < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Didn't get OMX_StateExecuting\n");
> +        return AVERROR_UNKNOWN;
> +    }
> +
> +    for (i = 0; i < s->num_out_buffers; i++)
> +        OMX_FillThisBuffer(s->handle, s->out_buffer_headers[i]);
> +    for (i = 0; i < s->num_in_buffers; i++)
> +        s->free_in_buffers[s->num_free_in_buffers++] = s->in_buffer_headers[i];
> +    return 0;
> +}
> +
> ...
> +
> +static int omx_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> +                            const AVFrame *frame, int *got_packet)
> +{
> +    OMXCodecContext *s = avctx->priv_data;
> +    int ret = 0;
> +    OMX_BUFFERHEADERTYPE* buffer;
> +
> +    if (frame) {
> +        uint8_t *dst[4];
> +        int linesize[4];
> +        pthread_mutex_lock(&s->input_mutex);
> +        while (!s->num_free_in_buffers)
> +            pthread_cond_wait(&s->input_cond, &s->input_mutex);
> +        buffer = s->free_in_buffers[--s->num_free_in_buffers];
> +        pthread_mutex_unlock(&s->input_mutex);
> +
> +        buffer->nFilledLen = av_image_fill_arrays(dst, linesize, buffer->pBuffer, avctx->pix_fmt, s->stride, s->plane_size, 1);
> +        av_image_copy(dst, linesize, (const uint8_t**) frame->data, frame->linesize, avctx->pix_fmt, avctx->width, avctx->height);
> +        buffer->nFlags = OMX_BUFFERFLAG_ENDOFFRAME;
> +        buffer->nOffset = 0;
> +        // Convert the timestamps to microseconds; some encoders can ignore
> +        // the framerate and do VFR bit allocation based on timestamps.
> +        buffer->nTimeStamp = to_omx_ticks(av_rescale_q(frame->pts, avctx->time_base, AV_TIME_BASE_Q));
> +        OMX_EmptyThisBuffer(s->handle, buffer);
> +        s->num_in_frames++;
> +    }
> +
> +
> +retry:
> +    pthread_mutex_lock(&s->output_mutex);
> +    if (!frame && s->num_out_frames < s->num_in_frames) {
> +        while (!s->num_done_out_buffers)
> +            pthread_cond_wait(&s->output_cond, &s->output_mutex);
> +    }
> +    if (s->num_done_out_buffers) {
> +        buffer = s->done_out_buffers[0];
> +        s->num_done_out_buffers--;
> +        memmove(&s->done_out_buffers[0], &s->done_out_buffers[1], s->num_done_out_buffers * sizeof(OMX_BUFFERHEADERTYPE*));
> +    } else {
> +        buffer = NULL;
> +    }
> +    pthread_mutex_unlock(&s->output_mutex);
> +
> +    if (buffer) {
> +        if (buffer->nFlags & OMX_BUFFERFLAG_CODECCONFIG && avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +            if ((ret = av_reallocp(&avctx->extradata, avctx->extradata_size + buffer->nFilledLen + FF_INPUT_BUFFER_PADDING_SIZE)) < 0) {
> +                avctx->extradata_size = 0;
> +                return ret;
> +            }
> +            memcpy(avctx->extradata + avctx->extradata_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
> +            memcpy(avctx->extradata + avctx->extradata_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);

Doubled line.

> +            avctx->extradata_size += buffer->nFilledLen;
> +            memset(avctx->extradata + avctx->extradata_size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
> +            OMX_FillThisBuffer(s->handle, buffer);
> +            goto retry;
> +        }
> +        if (!(buffer->nFlags & OMX_BUFFERFLAG_ENDOFFRAME)) {
> +            int newsize = s->output_buf_size + buffer->nFilledLen;
> +            if ((ret = av_reallocp(&s->output_buf, newsize)) < 0) {
> +                s->output_buf_size = 0;
> +                return ret;
> +            }
> +            memcpy(s->output_buf + s->output_buf_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
> +            s->output_buf_size += buffer->nFilledLen;
> +            OMX_FillThisBuffer(s->handle, buffer);
> +            goto retry;
> +        }
> +        s->num_out_frames++;
> +        if ((ret = ff_alloc_packet(pkt, s->output_buf_size + buffer->nFilledLen)) < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Error getting output packet of size %d.\n", (int)buffer->nFilledLen);
> +            OMX_FillThisBuffer(s->handle, buffer);
> +            return ret;
> +        }
> +        memcpy(pkt->data, s->output_buf, s->output_buf_size);
> +        memcpy(pkt->data + s->output_buf_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
> +        ret = s->output_buf_size + buffer->nFilledLen;
> +        av_freep(&s->output_buf);
> +        s->output_buf_size = 0;

This concatenation is rather nasty if it ever gets used nontrivially (multiple copies of the stream).  Is it only expected to be used for header data, or can it be used for the stream itself?  (Maybe when the output has multiple slices in H.264?)

> +        pkt->pts = av_rescale_q(from_omx_ticks(buffer->nTimeStamp), AV_TIME_BASE_Q, avctx->time_base);
> +        // We don't currently enable b-frames for the encoders, so set
> +        // pkt->dts = pkt->pts. (The calling code behaves worse if the encoder
> +        // doesn't set the dts).
> +        pkt->dts = pkt->pts;
> +        if (buffer->nFlags & OMX_BUFFERFLAG_SYNCFRAME)
> +            pkt->flags |= AV_PKT_FLAG_KEY;
> +        *got_packet = 1;
> +        OMX_FillThisBuffer(s->handle, buffer);
> +    }
> +    return ret;
> +}
> +
> ...
> +
> +static const AVClass omx_mpeg4enc_class = {
> +    .class_name = "OpenMAX IL MPEG4 encoder",

Most encoders have a short name here ("mpeg4_omx")?  (Maybe it doesn't actually matter.)

> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> ...

Is it reasonable to never check the return values of OMX_FTB and OMX_ETB calls?  The code will hang (in cond_wait) if this ever happens, though I don't know if it plausibly can in practice.

Also +1 to Diego's comment about casting in av_log calls, PRIu32 and friends would just be nicer.


Otherwise LGTM.

Thanks,

- Mark
Martin Storsjö April 9, 2016, 7:03 p.m. | #4
On Sat, 9 Apr 2016, Mark Thompson wrote:

> On 07/04/16 12:43, Martin Storsjö wrote:
>> ...
>> +
>> +static av_cold int omx_component_init(AVCodecContext *avctx, const char *role)
>> +{
>> +    OMXCodecContext *s = avctx->priv_data;
>> +    OMX_PARAM_COMPONENTROLETYPE role_params = { 0 };
>> +    OMX_PORT_PARAM_TYPE video_port_params = { 0 };
>> +    OMX_PARAM_PORTDEFINITIONTYPE in_port_params = { 0 }, out_port_params = { 0 };
>> +    OMX_VIDEO_PARAM_PORTFORMATTYPE video_port_format = { 0 };
>> +    OMX_VIDEO_PARAM_BITRATETYPE vid_param_bitrate = { 0 };
>> +    OMX_ERRORTYPE err;
>> +    int i, default_size;
>> +
>> +    s->version.s.nVersionMajor = 1;
>> +    s->version.s.nVersionMinor = 1;
>> +
>> +    err = omx_context->ptr_GetHandle(&s->handle, s->component_name, s, (OMX_CALLBACKTYPE*) &callbacks);
>> +    if (err != OMX_ErrorNone) {
>> +        av_log(avctx, AV_LOG_ERROR, "OMX_GetHandle(%s) failed: %x\n", s->component_name, err);
>> +        return AVERROR_UNKNOWN;
>> +    }
>> +
>> +    // This one crashes the mediaserver on qcom, if used over IOMX
>> +    INIT_STRUCT(role_params);
>> +    av_strlcpy(role_params.cRole, role, sizeof(role_params.cRole));
>> +    // Intentionally ignore errors on this one
>> +    OMX_SetParameter(s->handle, OMX_IndexParamStandardComponentRole, &role_params);
>> +
>> +    INIT_STRUCT(video_port_params);
>> +    err = OMX_GetParameter(s->handle, OMX_IndexParamVideoInit, &video_port_params);
>> +    CHECK(err);
>> +
>> +    s->in_port = s->out_port = -1;
>> +    for (i = 0; i < video_port_params.nPorts; i++) {
>> +        int port = video_port_params.nStartPortNumber + i;
>> +        OMX_PARAM_PORTDEFINITIONTYPE port_params = { 0 };
>> +        INIT_STRUCT(port_params);
>> +        port_params.nPortIndex = port;
>> +        err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &port_params);
>> +        if (err != OMX_ErrorNone) {
>> +            av_log(avctx, AV_LOG_WARNING, "port %d error %x\n", port, err);
>> +            break;
>> +        }
>> +        if (port_params.eDir == OMX_DirInput && s->in_port < 0) {
>> +            in_port_params = port_params;
>> +            s->in_port = port;
>> +        } else if (port_params.eDir == OMX_DirOutput && s->out_port < 0) {
>> +            out_port_params = port_params;
>> +            s->out_port = port;
>> +        }
>> +    }
>> +    if (s->in_port < 0 || s->out_port < 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "No in or out port found (in %d out %d)\n", s->in_port, s->out_port);
>> +        return AVERROR_UNKNOWN;
>> +    }
>> +
>> +    INIT_STRUCT(video_port_format);
>> +    video_port_format.nIndex = 0;
>> +    video_port_format.nPortIndex = s->in_port;
>> +    OMX_GetParameter(s->handle, OMX_IndexParamVideoPortFormat, &video_port_format);
>> +    s->color_format = video_port_format.eColorFormat;
>> +    // We only know the actual color format here, even though we've advertised the
>> +    // pixel format (YUV420P) via the encoder's pix_fmts array.
>> +    if (s->color_format != OMX_COLOR_FormatYUV420Planar &&
>> +        s->color_format != OMX_COLOR_FormatYUV420PackedPlanar) {
>> +        av_log(avctx, AV_LOG_ERROR, "Pixel format mismatch (%d isn't supported)\n", s->color_format);
>> +        return AVERROR_UNKNOWN;
>> +    }
>> +
>> +    in_port_params.bEnabled = OMX_TRUE;
>> +    in_port_params.bPopulated = OMX_FALSE;
>> +    in_port_params.eDomain = OMX_PortDomainVideo;
>> +    s->num_in_buffers = in_port_params.nBufferCountActual;
>> +
>> +    in_port_params.format.video.pNativeRender = NULL;
>> +    in_port_params.format.video.nStride = -1;
>> +    in_port_params.format.video.nSliceHeight = -1;
>
> These two writes are redundant.

Thanks, fixed. (Originally this was a shared encoder/decoder codepath, but 
I stripped out the decoder parts now when submitting.)

>> +    in_port_params.format.video.xFramerate = 30 << 16;
>> +    in_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
>> +    in_port_params.format.video.eColorFormat = s->color_format;
>> +    s->stride = avctx->width;
>> +    s->plane_size = avctx->height;
>> +    in_port_params.format.video.nStride = s->stride;
>> +    in_port_params.format.video.nSliceHeight = s->plane_size;
>> +    if (avctx->framerate.den > 0 && avctx->framerate.num > 0)
>> +        in_port_params.format.video.xFramerate = (1 << 16) * avctx->framerate.num / avctx->framerate.den;
>> +    else
>> +        in_port_params.format.video.xFramerate = (1 << 16) * avctx->time_base.den / avctx->time_base.num;
>> +    in_port_params.format.video.nFrameWidth = avctx->width;
>> +    in_port_params.format.video.nFrameHeight = avctx->height;
>> +    default_size = av_image_get_buffer_size(avctx->pix_fmt, s->stride, s->plane_size, 1);
>> +    if (in_port_params.nBufferSize < default_size)
>> +        in_port_params.nBufferSize = default_size;
>> +
>> +    err = OMX_SetParameter(s->handle, OMX_IndexParamPortDefinition, &in_port_params);
>> +    CHECK(err);
>> +    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &in_port_params);
>> +    CHECK(err);
>
> Aren't in_port_params...nStride/nSliceHeight possibly-updated here, if for example the component wants more alignment/padding than you are trying to give it?

Yes, good point, I'll amend that.

(There are some encoders that require you to set nStride/nSliceHeight to 
-1 when setting, so for those we'd possibly need to ignore it here as 
well. But that's in the realm of workarounds, not sensible defaults.)

>> +
>> +    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
>> +    out_port_params.bEnabled = OMX_TRUE;
>> +    out_port_params.bPopulated = OMX_FALSE;
>> +    out_port_params.eDomain = OMX_PortDomainVideo;
>> +    out_port_params.format.video.pNativeRender = NULL;
>> +    out_port_params.format.video.nFrameWidth = avctx->width;
>> +    out_port_params.format.video.nFrameHeight = avctx->height;
>> +    out_port_params.format.video.nStride = 0;
>> +    out_port_params.format.video.nSliceHeight = 0;
>> +    out_port_params.format.video.nBitrate = avctx->bit_rate;
>> +    out_port_params.format.video.xFramerate = in_port_params.format.video.xFramerate;
>> +    out_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
>> +    if (avctx->codec->id == AV_CODEC_ID_MPEG4)
>> +        out_port_params.format.video.eCompressionFormat = OMX_VIDEO_CodingMPEG4;
>> +    else if (avctx->codec->id == AV_CODEC_ID_H264)
>> +        out_port_params.format.video.eCompressionFormat = OMX_VIDEO_CodingAVC;
>> +    s->num_out_buffers = out_port_params.nBufferCountActual;
>> +
>> +    err = OMX_SetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
>> +    CHECK(err);
>> +    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
>> +    CHECK(err);
>> +
>> +    INIT_STRUCT(vid_param_bitrate);
>> +    vid_param_bitrate.nPortIndex = s->out_port;
>> +    vid_param_bitrate.eControlRate = OMX_Video_ControlRateVariable;
>> +    vid_param_bitrate.nTargetBitrate = avctx->bit_rate;
>> +    err = OMX_SetParameter(s->handle, OMX_IndexParamVideoBitrate, &vid_param_bitrate);
>> +    if (err != OMX_ErrorNone)
>> +        av_log(avctx, AV_LOG_WARNING, "Unable to set video bitrate parameter\n");
>> +
>> +    if (avctx->codec->id == AV_CODEC_ID_H264) {
>> +        OMX_VIDEO_PARAM_AVCTYPE avc = { 0 };
>> +        INIT_STRUCT(avc);
>> +        avc.nPortIndex = s->out_port;
>> +        err = OMX_GetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
>> +        CHECK(err);
>> +        avc.nBFrames = 0;
>> +        avc.nPFrames = avctx->gop_size - 1;
>> +        err = OMX_SetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
>> +        CHECK(err);
>> +    }
>> +
>> +    err = OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateIdle, NULL);
>> +    CHECK(err);
>> +
>> +    s->in_buffer_headers  = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
>> +    s->free_in_buffers    = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
>> +    s->out_buffer_headers = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
>> +    s->done_out_buffers   = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
>> +    if (!s->in_buffer_headers || !s->free_in_buffers || !s->out_buffer_headers || !s->done_out_buffers)
>> +        return AVERROR(ENOMEM);
>> +    for (i = 0; i < s->num_in_buffers && err == OMX_ErrorNone; i++)
>> +        err = OMX_AllocateBuffer(s->handle, &s->in_buffer_headers[i],  s->in_port,  s, in_port_params.nBufferSize);
>> +    CHECK(err);
>> +    s->num_in_buffers = i;
>> +    for (i = 0; i < s->num_out_buffers && err == OMX_ErrorNone; i++)
>> +        err = OMX_AllocateBuffer(s->handle, &s->out_buffer_headers[i], s->out_port, s, out_port_params.nBufferSize);
>> +    CHECK(err);
>> +    s->num_out_buffers = i;
>> +
>> +    if (wait_for_state(s, OMX_StateIdle) < 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "Didn't get OMX_StateIdle\n");
>> +        return AVERROR_UNKNOWN;
>> +    }
>> +    err = OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateExecuting, NULL);
>> +    CHECK(err);
>> +    if (wait_for_state(s, OMX_StateExecuting) < 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "Didn't get OMX_StateExecuting\n");
>> +        return AVERROR_UNKNOWN;
>> +    }
>> +
>> +    for (i = 0; i < s->num_out_buffers; i++)
>> +        OMX_FillThisBuffer(s->handle, s->out_buffer_headers[i]);
>> +    for (i = 0; i < s->num_in_buffers; i++)
>> +        s->free_in_buffers[s->num_free_in_buffers++] = s->in_buffer_headers[i];
>> +    return 0;
>> +}
>> +
>> ...
>> +
>> +static int omx_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>> +                            const AVFrame *frame, int *got_packet)
>> +{
>> +    OMXCodecContext *s = avctx->priv_data;
>> +    int ret = 0;
>> +    OMX_BUFFERHEADERTYPE* buffer;
>> +
>> +    if (frame) {
>> +        uint8_t *dst[4];
>> +        int linesize[4];
>> +        pthread_mutex_lock(&s->input_mutex);
>> +        while (!s->num_free_in_buffers)
>> +            pthread_cond_wait(&s->input_cond, &s->input_mutex);
>> +        buffer = s->free_in_buffers[--s->num_free_in_buffers];
>> +        pthread_mutex_unlock(&s->input_mutex);
>> +
>> +        buffer->nFilledLen = av_image_fill_arrays(dst, linesize, buffer->pBuffer, avctx->pix_fmt, s->stride, s->plane_size, 1);
>> +        av_image_copy(dst, linesize, (const uint8_t**) frame->data, frame->linesize, avctx->pix_fmt, avctx->width, avctx->height);
>> +        buffer->nFlags = OMX_BUFFERFLAG_ENDOFFRAME;
>> +        buffer->nOffset = 0;
>> +        // Convert the timestamps to microseconds; some encoders can ignore
>> +        // the framerate and do VFR bit allocation based on timestamps.
>> +        buffer->nTimeStamp = to_omx_ticks(av_rescale_q(frame->pts, avctx->time_base, AV_TIME_BASE_Q));
>> +        OMX_EmptyThisBuffer(s->handle, buffer);
>> +        s->num_in_frames++;
>> +    }
>> +
>> +
>> +retry:
>> +    pthread_mutex_lock(&s->output_mutex);
>> +    if (!frame && s->num_out_frames < s->num_in_frames) {
>> +        while (!s->num_done_out_buffers)
>> +            pthread_cond_wait(&s->output_cond, &s->output_mutex);
>> +    }
>> +    if (s->num_done_out_buffers) {
>> +        buffer = s->done_out_buffers[0];
>> +        s->num_done_out_buffers--;
>> +        memmove(&s->done_out_buffers[0], &s->done_out_buffers[1], s->num_done_out_buffers * sizeof(OMX_BUFFERHEADERTYPE*));
>> +    } else {
>> +        buffer = NULL;
>> +    }
>> +    pthread_mutex_unlock(&s->output_mutex);
>> +
>> +    if (buffer) {
>> +        if (buffer->nFlags & OMX_BUFFERFLAG_CODECCONFIG && avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
>> +            if ((ret = av_reallocp(&avctx->extradata, avctx->extradata_size + buffer->nFilledLen + FF_INPUT_BUFFER_PADDING_SIZE)) < 0) {
>> +                avctx->extradata_size = 0;
>> +                return ret;
>> +            }
>> +            memcpy(avctx->extradata + avctx->extradata_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
>> +            memcpy(avctx->extradata + avctx->extradata_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
>
> Doubled line.

Oops, fixed, thanks.

>> +            avctx->extradata_size += buffer->nFilledLen;
>> +            memset(avctx->extradata + avctx->extradata_size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
>> +            OMX_FillThisBuffer(s->handle, buffer);
>> +            goto retry;
>> +        }
>> +        if (!(buffer->nFlags & OMX_BUFFERFLAG_ENDOFFRAME)) {
>> +            int newsize = s->output_buf_size + buffer->nFilledLen;
>> +            if ((ret = av_reallocp(&s->output_buf, newsize)) < 0) {
>> +                s->output_buf_size = 0;
>> +                return ret;
>> +            }
>> +            memcpy(s->output_buf + s->output_buf_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
>> +            s->output_buf_size += buffer->nFilledLen;
>> +            OMX_FillThisBuffer(s->handle, buffer);
>> +            goto retry;
>> +        }
>> +        s->num_out_frames++;
>> +        if ((ret = ff_alloc_packet(pkt, s->output_buf_size + buffer->nFilledLen)) < 0) {
>> +            av_log(avctx, AV_LOG_ERROR, "Error getting output packet of size %d.\n", (int)buffer->nFilledLen);
>> +            OMX_FillThisBuffer(s->handle, buffer);
>> +            return ret;
>> +        }
>> +        memcpy(pkt->data, s->output_buf, s->output_buf_size);
>> +        memcpy(pkt->data + s->output_buf_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
>> +        ret = s->output_buf_size + buffer->nFilledLen;
>> +        av_freep(&s->output_buf);
>> +        s->output_buf_size = 0;
>
> This concatenation is rather nasty if it ever gets used nontrivially (multiple copies of the stream).  Is it only expected to be used for header data, or can it be used for the stream itself?  (Maybe when the output has multiple slices in H.264?)

It can happen if the output frame is larger than the current buffer size. 
On the raspberry pi, it seems to happen pretty often for IDR frames, 
although I haven't checked if it really is multiple slices or just one NAL 
unit cut into several buffers.

>> +        pkt->pts = av_rescale_q(from_omx_ticks(buffer->nTimeStamp), AV_TIME_BASE_Q, avctx->time_base);
>> +        // We don't currently enable b-frames for the encoders, so set
>> +        // pkt->dts = pkt->pts. (The calling code behaves worse if the encoder
>> +        // doesn't set the dts).
>> +        pkt->dts = pkt->pts;
>> +        if (buffer->nFlags & OMX_BUFFERFLAG_SYNCFRAME)
>> +            pkt->flags |= AV_PKT_FLAG_KEY;
>> +        *got_packet = 1;
>> +        OMX_FillThisBuffer(s->handle, buffer);
>> +    }
>> +    return ret;
>> +}
>> +
>> ...
>> +
>> +static const AVClass omx_mpeg4enc_class = {
>> +    .class_name = "OpenMAX IL MPEG4 encoder",
>
> Most encoders have a short name here ("mpeg4_omx")?  (Maybe it doesn't actually matter.)

Dunno if it matters, but I'll change it into mpeg4_omx/h264_omx.

>> +    .item_name  = av_default_item_name,
>> +    .option     = options,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> ...
>
> Is it reasonable to never check the return values of OMX_FTB and OMX_ETB calls?  The code will hang (in cond_wait) if this ever happens, though I don't know if it plausibly can in practice.

I guess one could check those calls as well, although I'm not sure how 
well one can recover (with proper cleanup) in such a case - I haven't 
really thought about it.

> Also +1 to Diego's comment about casting in av_log calls, PRIu32 and friends would just be nicer.

Ok, will fix.

// Martin
Anton Khirnov April 10, 2016, 6:48 a.m. | #5
Quoting Martin Storsjö (2016-04-07 13:43:47)
> ---
> Fixed most of wm4's comments, except for the static pixfmt init
> and getting rid of dlopen.
> ---
>  configure              |   6 +
>  libavcodec/Makefile    |   2 +
>  libavcodec/allcodecs.c |   2 +
>  libavcodec/omx.c       | 783 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 793 insertions(+)
>  create mode 100644 libavcodec/omx.c
> 
> diff --git a/libavcodec/omx.c b/libavcodec/omx.c
> new file mode 100644
> index 0000000..fbb6c72
> --- /dev/null
> +++ b/libavcodec/omx.c
> +
> +static OMXContext *omx_context;

Do you really need this? Can't you just dlopen independently in each
encoder instance?

> +
> +    in_port_params.format.video.pNativeRender = NULL;
> +    in_port_params.format.video.nStride = -1;
> +    in_port_params.format.video.nSliceHeight = -1;
> +    in_port_params.format.video.xFramerate = 30 << 16;

This is overwritten below, right?

> +static int omx_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> +                            const AVFrame *frame, int *got_packet)
> +{
> +    OMXCodecContext *s = avctx->priv_data;
> +    int ret = 0;
> +    OMX_BUFFERHEADERTYPE* buffer;
> +
> +    if (frame) {
> +        uint8_t *dst[4];
> +        int linesize[4];
> +        pthread_mutex_lock(&s->input_mutex);
> +        while (!s->num_free_in_buffers)
> +            pthread_cond_wait(&s->input_cond, &s->input_mutex);
> +        buffer = s->free_in_buffers[--s->num_free_in_buffers];
> +        pthread_mutex_unlock(&s->input_mutex);
> +
> +        buffer->nFilledLen = av_image_fill_arrays(dst, linesize, buffer->pBuffer, avctx->pix_fmt, s->stride, s->plane_size, 1);
> +        av_image_copy(dst, linesize, (const uint8_t**) frame->data, frame->linesize, avctx->pix_fmt, avctx->width, avctx->height);
> +        buffer->nFlags = OMX_BUFFERFLAG_ENDOFFRAME;
> +        buffer->nOffset = 0;
> +        // Convert the timestamps to microseconds; some encoders can ignore
> +        // the framerate and do VFR bit allocation based on timestamps.
> +        buffer->nTimeStamp = to_omx_ticks(av_rescale_q(frame->pts, avctx->time_base, AV_TIME_BASE_Q));
> +        OMX_EmptyThisBuffer(s->handle, buffer);
> +        s->num_in_frames++;
> +    }
> +
> +
> +retry:

I find such backward gotos really ugly. I think it'd be much nicer if
you split this whole block into a separate function that can be called
multiple times.

> +    if (buffer) {
> +        if (buffer->nFlags & OMX_BUFFERFLAG_CODECCONFIG && avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +            if ((ret = av_reallocp(&avctx->extradata, avctx->extradata_size + buffer->nFilledLen + FF_INPUT_BUFFER_PADDING_SIZE)) < 0) {
                                                                                                      ^^
It's AV_ now. Same in other places.
Martin Storsjö April 10, 2016, 8:38 a.m. | #6
On Sun, 10 Apr 2016, Anton Khirnov wrote:

> Quoting Martin Storsjö (2016-04-07 13:43:47)
>> ---
>> Fixed most of wm4's comments, except for the static pixfmt init
>> and getting rid of dlopen.
>> ---
>>  configure              |   6 +
>>  libavcodec/Makefile    |   2 +
>>  libavcodec/allcodecs.c |   2 +
>>  libavcodec/omx.c       | 783 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 793 insertions(+)
>>  create mode 100644 libavcodec/omx.c
>> 
>> diff --git a/libavcodec/omx.c b/libavcodec/omx.c
>> new file mode 100644
>> index 0000000..fbb6c72
>> --- /dev/null
>> +++ b/libavcodec/omx.c
>> +
>> +static OMXContext *omx_context;
>
> Do you really need this? Can't you just dlopen independently in each
> encoder instance?

I'm a little unsure. First off, it's not so much about dlopening it 
multiple times, but the global OMX_Init function. With well-behaved OMX 
drivers, the init function might be reference counted, so multiple nested 
init/deinit do what you want them to, but I don't think there's any such 
guarantee. (In particular, I know I've seen drivers where this doesn't 
work.) Of course, with such a driver, you're screwed if your calling app 
(or another lib) wants to use OMX for something else at the same time.

>> +
>> +    in_port_params.format.video.pNativeRender = NULL;
>> +    in_port_params.format.video.nStride = -1;
>> +    in_port_params.format.video.nSliceHeight = -1;
>> +    in_port_params.format.video.xFramerate = 30 << 16;
>
> This is overwritten below, right?

Good catch, yes. Mark pointed out that nStride/nSliceHeight was 
duplicated, but xFramerate apparently also is.

>> +static int omx_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>> +                            const AVFrame *frame, int *got_packet)
>> +{
>> +    OMXCodecContext *s = avctx->priv_data;
>> +    int ret = 0;
>> +    OMX_BUFFERHEADERTYPE* buffer;
>> +
>> +    if (frame) {
>> +        uint8_t *dst[4];
>> +        int linesize[4];
>> +        pthread_mutex_lock(&s->input_mutex);
>> +        while (!s->num_free_in_buffers)
>> +            pthread_cond_wait(&s->input_cond, &s->input_mutex);
>> +        buffer = s->free_in_buffers[--s->num_free_in_buffers];
>> +        pthread_mutex_unlock(&s->input_mutex);
>> +
>> +        buffer->nFilledLen = av_image_fill_arrays(dst, linesize, buffer->pBuffer, avctx->pix_fmt, s->stride, s->plane_size, 1);
>> +        av_image_copy(dst, linesize, (const uint8_t**) frame->data, frame->linesize, avctx->pix_fmt, avctx->width, avctx->height);
>> +        buffer->nFlags = OMX_BUFFERFLAG_ENDOFFRAME;
>> +        buffer->nOffset = 0;
>> +        // Convert the timestamps to microseconds; some encoders can ignore
>> +        // the framerate and do VFR bit allocation based on timestamps.
>> +        buffer->nTimeStamp = to_omx_ticks(av_rescale_q(frame->pts, avctx->time_base, AV_TIME_BASE_Q));
>> +        OMX_EmptyThisBuffer(s->handle, buffer);
>> +        s->num_in_frames++;
>> +    }
>> +
>> +
>> +retry:
>
> I find such backward gotos really ugly. I think it'd be much nicer if
> you split this whole block into a separate function that can be called
> multiple times.

Ok, will try.

>> +    if (buffer) {
>> +        if (buffer->nFlags & OMX_BUFFERFLAG_CODECCONFIG && avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
>> +            if ((ret = av_reallocp(&avctx->extradata, avctx->extradata_size + buffer->nFilledLen + FF_INPUT_BUFFER_PADDING_SIZE)) < 0) {
>                                                                                                      ^^
> It's AV_ now. Same in other places.

Thanks, fixed locally.

// Martin
Martin Storsjö April 10, 2016, 8:59 a.m. | #7
On Sat, 9 Apr 2016, Martin Storsjö wrote:

> On Sat, 9 Apr 2016, Mark Thompson wrote:
>
>> On 07/04/16 12:43, Martin Storsjö wrote:
>>> ...
>>> +    in_port_params.format.video.xFramerate = 30 << 16;
>>> +    in_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
>>> +    in_port_params.format.video.eColorFormat = s->color_format;
>>> +    s->stride = avctx->width;
>>> +    s->plane_size = avctx->height;
>>> +    in_port_params.format.video.nStride = s->stride;
>>> +    in_port_params.format.video.nSliceHeight = s->plane_size;
>>> +    if (avctx->framerate.den > 0 && avctx->framerate.num > 0)
>>> +        in_port_params.format.video.xFramerate = (1 << 16) * 
>>> avctx->framerate.num / avctx->framerate.den;
>>> +    else
>>> +        in_port_params.format.video.xFramerate = (1 << 16) * 
>>> avctx->time_base.den / avctx->time_base.num;
>>> +    in_port_params.format.video.nFrameWidth = avctx->width;
>>> +    in_port_params.format.video.nFrameHeight = avctx->height;
>>> +    default_size = av_image_get_buffer_size(avctx->pix_fmt, s->stride, 
>>> s->plane_size, 1);
>>> +    if (in_port_params.nBufferSize < default_size)
>>> +        in_port_params.nBufferSize = default_size;
>>> +
>>> +    err = OMX_SetParameter(s->handle, OMX_IndexParamPortDefinition, 
>>> &in_port_params);
>>> +    CHECK(err);
>>> +    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, 
>>> &in_port_params);
>>> +    CHECK(err);
>> 
>> Aren't in_port_params...nStride/nSliceHeight possibly-updated here, if for 
>> example the component wants more alignment/padding than you are trying to 
>> give it?
>
> Yes, good point, I'll amend that.

Thanks for pointing this out, this allows getting rid of the manual 
alignment for the raspberry pi as well. Dunno why I never thought of that 
instead of just manually overriding the alignment.

// Martin
Martin Storsjö April 10, 2016, 9:07 a.m. | #8
On Thu, 7 Apr 2016, Diego Biurrun wrote:

> On Thu, Apr 07, 2016 at 02:43:47PM +0300, Martin Storsjö wrote:
>
>> +{
>> +    OMXCodecContext *s = app_data;
>> +    switch (event) {
>> +    case OMX_EventError:
>> +        pthread_mutex_lock(&s->state_mutex);
>> +        av_log(s->avctx, AV_LOG_ERROR, "OMX error %x\n", (int) data1);
>
> Maybe use Posix integer printf specifiers instead of casting?
> In any case unsigned looks like a better choice than signed int; I
> assume that OMX_U32 is an unsigned type.

This actually doesn't work:

libavcodec/omx.c: In function ‘event_handler’:
libavcodec/omx.c:221:9: warning: format ‘%x’ expects argument of type 
‘unsigned int’, but argument 4 has type ‘OMX_U32’ [-Wformat=]
          av_log(s->avctx, AV_LOG_ERROR, "OMX error %"PRIx32"\n", data1);

In OMX_Types.h (which you can find e.g. here: 
https://www.khronos.org/registry/omxil/) you'll find this gem:

/** OMX_U32 is a 32 bit unsigned quantity that is 32 bit word aligned */
typedef unsigned long OMX_U32;

So apparently OMX as it stands is never used in 64 bit processes, at least 
without hacking the official headers.

Given this, what's the least ugly way to handle it? Cast to uint32_t and 
print using PRIx32?

// Martin
Mark Thompson April 10, 2016, 9:39 a.m. | #9
On 10/04/16 10:07, Martin Storsjö wrote:
> On Thu, 7 Apr 2016, Diego Biurrun wrote:
> 
>> On Thu, Apr 07, 2016 at 02:43:47PM +0300, Martin Storsjö wrote:
>>
>>> +{
>>> +    OMXCodecContext *s = app_data;
>>> +    switch (event) {
>>> +    case OMX_EventError:
>>> +        pthread_mutex_lock(&s->state_mutex);
>>> +        av_log(s->avctx, AV_LOG_ERROR, "OMX error %x\n", (int) data1);
>>
>> Maybe use Posix integer printf specifiers instead of casting?
>> In any case unsigned looks like a better choice than signed int; I
>> assume that OMX_U32 is an unsigned type.
> 
> This actually doesn't work:
> 
> libavcodec/omx.c: In function ‘event_handler’:
> libavcodec/omx.c:221:9: warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 4 has type ‘OMX_U32’ [-Wformat=]
>          av_log(s->avctx, AV_LOG_ERROR, "OMX error %"PRIx32"\n", data1);
> 
> In OMX_Types.h (which you can find e.g. here: https://www.khronos.org/registry/omxil/) you'll find this gem:
> 
> /** OMX_U32 is a 32 bit unsigned quantity that is 32 bit word aligned */
> typedef unsigned long OMX_U32;

How wonderful.

> So apparently OMX as it stands is never used in 64 bit processes, at least without hacking the official headers.
> 
> Given this, what's the least ugly way to handle it? Cast to uint32_t and print using PRIx32?

Well, given that the standard says it has to be unsigned long just use %lx?

(It can't change without a new official header and a corresponding version bump, so I don't think that's unreasonable.)
Martin Storsjö April 10, 2016, 6:33 p.m. | #10
On Sun, 10 Apr 2016, Mark Thompson wrote:

> On 10/04/16 10:07, Martin Storsjö wrote:
>> On Thu, 7 Apr 2016, Diego Biurrun wrote:
>> 
>>> On Thu, Apr 07, 2016 at 02:43:47PM +0300, Martin Storsjö wrote:
>>>
>>>> +{
>>>> +    OMXCodecContext *s = app_data;
>>>> +    switch (event) {
>>>> +    case OMX_EventError:
>>>> +        pthread_mutex_lock(&s->state_mutex);
>>>> +        av_log(s->avctx, AV_LOG_ERROR, "OMX error %x\n", (int) data1);
>>>
>>> Maybe use Posix integer printf specifiers instead of casting?
>>> In any case unsigned looks like a better choice than signed int; I
>>> assume that OMX_U32 is an unsigned type.
>> 
>> This actually doesn't work:
>> 
>> libavcodec/omx.c: In function ‘event_handler’:
>> libavcodec/omx.c:221:9: warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 4 has type ‘OMX_U32’ [-Wformat=]
>>          av_log(s->avctx, AV_LOG_ERROR, "OMX error %"PRIx32"\n", data1);
>> 
>> In OMX_Types.h (which you can find e.g. here: https://www.khronos.org/registry/omxil/) you'll find this gem:
>> 
>> /** OMX_U32 is a 32 bit unsigned quantity that is 32 bit word aligned */
>> typedef unsigned long OMX_U32;
>
> How wonderful.
>
>> So apparently OMX as it stands is never used in 64 bit processes, at least without hacking the official headers.
>> 
>> Given this, what's the least ugly way to handle it? Cast to uint32_t and print using PRIx32?
>
> Well, given that the standard says it has to be unsigned long just use %lx?
>
> (It can't change without a new official header and a corresponding 
> version bump, so I don't think that's unreasonable.)

Well, even if the standard can't change, actual implementations can - see 
e.g. 
https://android.googlesource.com/platform/frameworks/native/+/849de60c1eae2ec28f0b468b9ec16b339aad17e9%5E%21/

(To work with such a setup, one would need to have the exact OMX headers 
used by the system, not the public standard ones though.)

Although, by using %lx, the only thing that would break is our log 
messages (that aren't printed by default). So I don't really oppose it 
much, but given the precarious situation with this typedef, I'd prefer a 
uint32_t cast and PRIx32.

// Martin
Vittorio Giovara April 11, 2016, 4:20 p.m. | #11
On Thu, Apr 7, 2016 at 3:17 PM, Martin Storsjö <martin@martin.st> wrote:
> On Thu, 7 Apr 2016, Diego Biurrun wrote:
>
>>> +static av_cold void *dlsym2(void *handle, const char *symbol, const char
>>> *prefix)
>>
>>
>> 2?
>
>
> Appending 2 is our own normal naming scheme for "new and improved version of
> function X" :P
>
> Perhaps dlsym_prefix would be more descriptive, even if it places more
> emphasis than wanted on using a prefix, which is used very seldom.
>
>>> +    s->in_buffer_headers  = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>> s->num_in_buffers);
>>> +    s->free_in_buffers    = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>> s->num_in_buffers);
>>> +    s->out_buffer_headers = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>> s->num_out_buffers);
>>> +    s->done_out_buffers   = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>> s->num_out_buffers);
>>> +    if (!s->in_buffer_headers || !s->free_in_buffers ||
>>> !s->out_buffer_headers || !s->done_out_buffers)
>>> +        return AVERROR(ENOMEM);
>>
>>
>> Why is this not a memleak if some of the mallocs fail?
>
>
> Because if this function returns an error, the caller (omx_encode_init) will
> goto fail and call cleanup, which cleans up any and all allocations.

Would it matter if FF_CODEC_CAP_INIT_CLEANUP was implemented instead?
Also is this decoder a candidate for FF_CODEC_CAP_INIT_THREADSAFE?
Martin Storsjö April 11, 2016, 7:36 p.m. | #12
On Mon, 11 Apr 2016, Vittorio Giovara wrote:

> On Thu, Apr 7, 2016 at 3:17 PM, Martin Storsjö <martin@martin.st> wrote:
>> On Thu, 7 Apr 2016, Diego Biurrun wrote:
>>
>>>> +static av_cold void *dlsym2(void *handle, const char *symbol, const char
>>>> *prefix)
>>>
>>>
>>> 2?
>>
>>
>> Appending 2 is our own normal naming scheme for "new and improved version of
>> function X" :P
>>
>> Perhaps dlsym_prefix would be more descriptive, even if it places more
>> emphasis than wanted on using a prefix, which is used very seldom.
>>
>>>> +    s->in_buffer_headers  = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>>> s->num_in_buffers);
>>>> +    s->free_in_buffers    = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>>> s->num_in_buffers);
>>>> +    s->out_buffer_headers = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>>> s->num_out_buffers);
>>>> +    s->done_out_buffers   = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) *
>>>> s->num_out_buffers);
>>>> +    if (!s->in_buffer_headers || !s->free_in_buffers ||
>>>> !s->out_buffer_headers || !s->done_out_buffers)
>>>> +        return AVERROR(ENOMEM);
>>>
>>>
>>> Why is this not a memleak if some of the mallocs fail?
>>
>>
>> Because if this function returns an error, the caller (omx_encode_init) will
>> goto fail and call cleanup, which cleans up any and all allocations.
>
> Would it matter if FF_CODEC_CAP_INIT_CLEANUP was implemented instead?

It wouldn't matter much, it would remove one line of code here. The code 
is designed in the same way as that, but prior to the capability.

> Also is this decoder a candidate for FF_CODEC_CAP_INIT_THREADSAFE?

Yes. Also, it's an encoder, not a decoder.

// Martin

Patch

diff --git a/configure b/configure
index 546ddc0..4a3e570 100755
--- a/configure
+++ b/configure
@@ -225,6 +225,7 @@  External library support:
                            native MPEG-4/Xvid encoder exists [no]
   --enable-mmal            enable decoding via MMAL [no]
   --enable-nvenc           enable encoding via NVENC [no]
+  --enable-omx             enable encoding via OpenMAX IL [no]
   --enable-openssl         enable openssl [no]
   --enable-x11grab         enable X11 grabbing (legacy) [no]
   --enable-zlib            enable zlib [autodetect]
@@ -1252,6 +1253,7 @@  EXTERNAL_LIBRARY_LIST="
     libxvid
     mmal
     nvenc
+    omx
     openssl
     x11grab
     zlib
@@ -1878,6 +1880,7 @@  mpegvideo_select="blockdsp hpeldsp idctdsp me_cmp mpeg_er videodsp"
 mpegvideoenc_select="me_cmp mpegvideo pixblockdsp qpeldsp"
 nvenc_deps_any="dlopen LoadLibrary"
 nvenc_extralibs='$ldl'
+omx_extralibs='$ldl'
 qsvdec_select="qsv"
 qsvenc_select="qsv"
 vaapi_encode_deps="vaapi"
@@ -1954,6 +1957,7 @@  h263p_encoder_select="h263_encoder"
 h264_decoder_select="cabac golomb h264chroma h264dsp h264pred h264qpel videodsp"
 h264_decoder_suggest="error_resilience"
 h264_nvenc_encoder_deps="nvenc"
+h264_omx_encoder_deps="omx dlopen pthreads"
 h264_qsv_decoder_deps="libmfx"
 h264_qsv_decoder_select="h264_mp4toannexb_bsf h264_parser qsvdec h264_qsv_hwaccel"
 h264_qsv_encoder_deps="libmfx"
@@ -2019,6 +2023,7 @@  mpeg2_qsv_encoder_deps="libmfx"
 mpeg2_qsv_encoder_select="qsvenc"
 mpeg4_decoder_select="h263_decoder mpeg4video_parser"
 mpeg4_encoder_select="h263_encoder"
+mpeg4_omx_encoder_deps="omx dlopen pthreads"
 msa1_decoder_select="mss34dsp"
 msmpeg4v1_decoder_select="h263_decoder"
 msmpeg4v2_decoder_select="h263_decoder"
@@ -4603,6 +4608,7 @@  enabled mmal              && { check_lib interface/mmal/mmal.h mmal_port_connect
                                     check_lib interface/mmal/mmal.h mmal_port_connect ; }
                                 check_lib interface/mmal/mmal.h mmal_port_connect ; } ||
                                die "ERROR: mmal not found"; }
+enabled omx               && { check_header OMX_Core.h || die "ERROR: OpenMAX IL headers not found"; }
 enabled openssl           && { check_pkg_config openssl openssl/ssl.h SSL_library_init && {
                                add_cflags $openssl_cflags && add_extralibs $openssl_libs; }||
                                check_lib openssl/ssl.h SSL_library_init -lssl -lcrypto ||
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 22279fb..452cb9d 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -257,6 +257,7 @@  OBJS-$(CONFIG_H264_DECODER)            += h264.o h264_cabac.o h264_cavlc.o \
                                           h2645_parse.o
 OBJS-$(CONFIG_H264_MMAL_DECODER)       += mmaldec.o
 OBJS-$(CONFIG_H264_NVENC_ENCODER)      += nvenc_h264.o
+OBJS-$(CONFIG_H264_OMX_ENCODER)        += omx.o
 OBJS-$(CONFIG_H264_QSV_DECODER)        += qsvdec_h2645.o
 OBJS-$(CONFIG_H264_QSV_ENCODER)        += qsvenc_h264.o
 OBJS-$(CONFIG_H264_VAAPI_ENCODER)      += vaapi_encode_h264.o vaapi_encode_h26x.o
@@ -330,6 +331,7 @@  OBJS-$(CONFIG_MPEG2VIDEO_ENCODER)      += mpeg12enc.o mpeg12.o
 OBJS-$(CONFIG_MPEG2_QSV_DECODER)       += qsvdec_mpeg2.o
 OBJS-$(CONFIG_MPEG2_QSV_ENCODER)       += qsvenc_mpeg2.o
 OBJS-$(CONFIG_MPEG4_DECODER)           += xvididct.o
+OBJS-$(CONFIG_MPEG4_OMX_ENCODER)       += omx.o
 OBJS-$(CONFIG_MSMPEG4V1_DECODER)       += msmpeg4dec.o msmpeg4.o msmpeg4data.o
 OBJS-$(CONFIG_MSMPEG4V2_DECODER)       += msmpeg4dec.o msmpeg4.o msmpeg4data.o
 OBJS-$(CONFIG_MSMPEG4V2_ENCODER)       += msmpeg4enc.o msmpeg4.o msmpeg4data.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 5446ec3..16e9cd6 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -487,11 +487,13 @@  void avcodec_register_all(void)
      * above is available */
     REGISTER_ENCODER(LIBOPENH264,       libopenh264);
     REGISTER_ENCODER(H264_NVENC,        h264_nvenc);
+    REGISTER_ENCODER(H264_OMX,          h264_omx);
     REGISTER_ENCODER(H264_QSV,          h264_qsv);
     REGISTER_ENCODER(LIBKVAZAAR,        libkvazaar);
     REGISTER_ENCODER(HEVC_NVENC,        hevc_nvenc);
     REGISTER_ENCODER(HEVC_QSV,          hevc_qsv);
     REGISTER_ENCODER(MPEG2_QSV,         mpeg2_qsv);
+    REGISTER_ENCODER(MPEG4_OMX,         mpeg4_omx);
 #if FF_API_NVENC_OLD_NAME
     REGISTER_ENCODER(NVENC_H264,        nvenc_h264);
     REGISTER_ENCODER(NVENC_HEVC,        nvenc_hevc);
diff --git a/libavcodec/omx.c b/libavcodec/omx.c
new file mode 100644
index 0000000..fbb6c72
--- /dev/null
+++ b/libavcodec/omx.c
@@ -0,0 +1,783 @@ 
+/*
+ * OMX Video encoder
+ * Copyright (C) 2011 Martin Storsjo
+ *
+ * This file is part of Libav.
+ *
+ * Libav is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * Libav is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Libav; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <dlfcn.h>
+#include <OMX_Core.h>
+#include <OMX_Component.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/time.h>
+
+#include "libavutil/avstring.h"
+#include "libavutil/avutil.h"
+#include "libavutil/common.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/log.h"
+#include "libavutil/opt.h"
+
+#include "avcodec.h"
+#include "h264.h"
+#include "internal.h"
+
+#ifdef OMX_SKIP64BIT
+static OMX_TICKS to_omx_ticks(int64_t value) {
+    OMX_TICKS s;
+    s.nLowPart = value;
+    s.nHighPart = value >> 32;
+    return s;
+}
+static int64_t from_omx_ticks(OMX_TICKS value) {
+    return (((int64_t)value.nHighPart) << 32) | value.nLowPart;
+}
+#else
+#define to_omx_ticks(x) (x)
+#define from_omx_ticks(x) (x)
+#endif
+
+#define INIT_STRUCT(x) do {                                               \
+        x.nSize = sizeof(x);                                              \
+        x.nVersion = s->version;                                          \
+    } while (0)
+#define CHECK(x) do {                                                     \
+        if (x != OMX_ErrorNone) {                                         \
+            av_log(avctx, AV_LOG_ERROR,                                   \
+                   "err %x (%d) on line %d\n", x, x, __LINE__);           \
+            return AVERROR_UNKNOWN;                                       \
+        }                                                                 \
+    } while (0)
+
+typedef struct OMXContext {
+    int users;
+    void *lib;
+    OMX_ERRORTYPE (*ptr_Init)(void);
+    OMX_ERRORTYPE (*ptr_Deinit)(void);
+    OMX_ERRORTYPE (*ptr_ComponentNameEnum)(OMX_STRING, OMX_U32, OMX_U32);
+    OMX_ERRORTYPE (*ptr_GetHandle)(OMX_HANDLETYPE*, OMX_STRING, OMX_PTR, OMX_CALLBACKTYPE*);
+    OMX_ERRORTYPE (*ptr_FreeHandle)(OMX_HANDLETYPE);
+    OMX_ERRORTYPE (*ptr_GetComponentsOfRole)(OMX_STRING, OMX_U32*, OMX_U8**);
+    OMX_ERRORTYPE (*ptr_GetRolesOfComponent)(OMX_STRING, OMX_U32*, OMX_U8**);
+} OMXContext;
+
+static OMXContext *omx_context;
+static pthread_mutex_t omx_context_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+static av_cold void *dlsym2(void *handle, const char *symbol, const char *prefix)
+{
+    char buf[50];
+    snprintf(buf, sizeof(buf), "%s%s", prefix ? prefix : "", symbol);
+    return dlsym(handle, buf);
+}
+
+static av_cold int omx_try_load(void *logctx, const char *libname, const char *prefix)
+{
+    OMXContext *s = omx_context;
+    s->lib = dlopen(libname, RTLD_NOW | RTLD_GLOBAL);
+    if (!s->lib) {
+        av_log(logctx, AV_LOG_WARNING, "%s not found\n", libname);
+        return AVERROR_ENCODER_NOT_FOUND;
+    }
+    s->ptr_Init                = dlsym2(s->lib, "OMX_Init", prefix);
+    s->ptr_Deinit              = dlsym2(s->lib, "OMX_Deinit", prefix);
+    s->ptr_ComponentNameEnum   = dlsym2(s->lib, "OMX_ComponentNameEnum", prefix);
+    s->ptr_GetHandle           = dlsym2(s->lib, "OMX_GetHandle", prefix);
+    s->ptr_FreeHandle          = dlsym2(s->lib, "OMX_FreeHandle", prefix);
+    s->ptr_GetComponentsOfRole = dlsym2(s->lib, "OMX_GetComponentsOfRole", prefix);
+    s->ptr_GetRolesOfComponent = dlsym2(s->lib, "OMX_GetRolesOfComponent", prefix);
+    if (!s->ptr_Init || !s->ptr_Deinit || !s->ptr_ComponentNameEnum ||
+        !s->ptr_GetHandle || !s->ptr_FreeHandle ||
+        !s->ptr_GetComponentsOfRole || !s->ptr_GetRolesOfComponent) {
+        av_log(logctx, AV_LOG_WARNING, "Not all functions found in %s\n", libname);
+        dlclose(s->lib);
+        s->lib = NULL;
+        return AVERROR_ENCODER_NOT_FOUND;
+    }
+    return 0;
+}
+
+static av_cold int omx_init(void *logctx, const char *libname, const char *prefix) {
+    static const char * const libnames[] = {
+        "libOMX_Core.so",
+        "libOmxCore.so",
+        NULL
+    };
+    const char* const* nameptr;
+    int ret = AVERROR_ENCODER_NOT_FOUND;
+
+    pthread_mutex_lock(&omx_context_mutex);
+    if (omx_context) {
+        omx_context->users++;
+        pthread_mutex_unlock(&omx_context_mutex);
+        return 0;
+    }
+
+    omx_context = av_mallocz(sizeof(*omx_context));
+    if (!omx_context) {
+        pthread_mutex_unlock(&omx_context_mutex);
+        return AVERROR(ENOMEM);
+    }
+    omx_context->users = 1;
+    if (libname) {
+        ret = omx_try_load(logctx, libname, prefix);
+        if (ret < 0) {
+            pthread_mutex_unlock(&omx_context_mutex);
+            return ret;
+        }
+    } else {
+        for (nameptr = libnames; *nameptr; nameptr++)
+            if (!(ret = omx_try_load(logctx, *nameptr, prefix)))
+                break;
+        if (!*nameptr) {
+            pthread_mutex_unlock(&omx_context_mutex);
+            return ret;
+        }
+    }
+
+    omx_context->ptr_Init();
+    pthread_mutex_unlock(&omx_context_mutex);
+    return 0;
+}
+
+static av_cold void omx_deinit(void) {
+    pthread_mutex_lock(&omx_context_mutex);
+    if (!omx_context) {
+        pthread_mutex_unlock(&omx_context_mutex);
+        return;
+    }
+    omx_context->users--;
+    if (!omx_context->users) {
+        omx_context->ptr_Deinit();
+        dlclose(omx_context->lib);
+        av_freep(&omx_context);
+    }
+    pthread_mutex_unlock(&omx_context_mutex);
+}
+
+typedef struct OMXCodecContext {
+    const AVClass *class;
+    char *libname;
+    char *libprefix;
+
+    AVCodecContext *avctx;
+
+    char component_name[OMX_MAX_STRINGNAME_SIZE];
+    OMX_VERSIONTYPE version;
+    OMX_HANDLETYPE handle;
+    int in_port, out_port;
+    OMX_COLOR_FORMATTYPE color_format;
+    int stride, plane_size;
+
+    int num_in_buffers, num_out_buffers;
+    OMX_BUFFERHEADERTYPE **in_buffer_headers;
+    OMX_BUFFERHEADERTYPE **out_buffer_headers;
+    int num_free_in_buffers;
+    OMX_BUFFERHEADERTYPE **free_in_buffers;
+    int num_done_out_buffers;
+    OMX_BUFFERHEADERTYPE **done_out_buffers;
+    pthread_mutex_t input_mutex;
+    pthread_cond_t input_cond;
+    pthread_mutex_t output_mutex;
+    pthread_cond_t output_cond;
+
+    pthread_mutex_t state_mutex;
+    pthread_cond_t state_cond;
+    OMX_STATETYPE state;
+    OMX_ERRORTYPE error;
+
+    int num_in_frames, num_out_frames;
+
+    uint8_t *output_buf;
+    int output_buf_size;
+} OMXCodecContext;
+
+static OMX_ERRORTYPE event_handler(OMX_HANDLETYPE component, OMX_PTR app_data, OMX_EVENTTYPE event, OMX_U32 data1, OMX_U32 data2, OMX_PTR event_data)
+{
+    OMXCodecContext *s = app_data;
+    switch (event) {
+    case OMX_EventError:
+        pthread_mutex_lock(&s->state_mutex);
+        av_log(s->avctx, AV_LOG_ERROR, "OMX error %x\n", (int) data1);
+        s->error = data1;
+        pthread_cond_broadcast(&s->state_cond);
+        pthread_mutex_unlock(&s->state_mutex);
+        break;
+    case OMX_EventCmdComplete:
+        if (data1 == OMX_CommandStateSet) {
+            pthread_mutex_lock(&s->state_mutex);
+            s->state = data2;
+            av_log(s->avctx, AV_LOG_VERBOSE, "OMX state changed to %d\n", (int) data2);
+            pthread_cond_broadcast(&s->state_cond);
+            pthread_mutex_unlock(&s->state_mutex);
+        } else if (data1 == OMX_CommandPortDisable) {
+            av_log(s->avctx, AV_LOG_VERBOSE, "OMX port %d disabled\n", (int) data2);
+        } else if (data1 == OMX_CommandPortEnable) {
+            av_log(s->avctx, AV_LOG_VERBOSE, "OMX port %d enabled\n", (int) data2);
+        } else {
+            av_log(s->avctx, AV_LOG_VERBOSE, "OMX command complete, command %d, value %d\n", (int) data1, (int) data2);
+        }
+        break;
+    case OMX_EventPortSettingsChanged:
+        av_log(s->avctx, AV_LOG_VERBOSE, "OMX port %d settings changed\n", (int) data1);
+        break;
+    default:
+        av_log(s->avctx, AV_LOG_VERBOSE, "OMX event %d %x %x\n", event, (int) data1, (int) data2);
+        break;
+    }
+    return OMX_ErrorNone;
+}
+
+static OMX_ERRORTYPE empty_buffer_done(OMX_HANDLETYPE component, OMX_PTR app_data, OMX_BUFFERHEADERTYPE *buffer)
+{
+    OMXCodecContext *s = app_data;
+    pthread_mutex_lock(&s->input_mutex);
+    s->free_in_buffers[s->num_free_in_buffers++] = buffer;
+    pthread_cond_broadcast(&s->input_cond);
+    pthread_mutex_unlock(&s->input_mutex);
+    return OMX_ErrorNone;
+}
+
+static OMX_ERRORTYPE fill_buffer_done(OMX_HANDLETYPE component, OMX_PTR app_data, OMX_BUFFERHEADERTYPE *buffer)
+{
+    OMXCodecContext *s = app_data;
+    pthread_mutex_lock(&s->output_mutex);
+    s->done_out_buffers[s->num_done_out_buffers++] = buffer;
+    pthread_cond_broadcast(&s->output_cond);
+    pthread_mutex_unlock(&s->output_mutex);
+    return OMX_ErrorNone;
+}
+
+static const OMX_CALLBACKTYPE callbacks = {
+    event_handler,
+    empty_buffer_done,
+    fill_buffer_done
+};
+
+static av_cold int find_component(void *logctx, const char *role, char *str,
+                                  int str_size)
+{
+    OMX_U32 i, num = 0;
+    char **components;
+    int ret = 0;
+
+    omx_context->ptr_GetComponentsOfRole((OMX_STRING) role, &num, NULL);
+    if (!num) {
+        av_log(logctx, AV_LOG_WARNING, "No component for role %s found\n", role);
+        return AVERROR_ENCODER_NOT_FOUND;
+    }
+    components = av_mallocz(sizeof(char*) * num);
+    if (!components)
+        return AVERROR(ENOMEM);
+    for (i = 0; i < num; i++) {
+        components[i] = av_mallocz(OMX_MAX_STRINGNAME_SIZE);
+        if (!components) {
+            ret = AVERROR(ENOMEM);
+            goto end;
+        }
+    }
+    omx_context->ptr_GetComponentsOfRole((OMX_STRING) role, &num, (OMX_U8**) components);
+    av_strlcpy(str, components[0], str_size);
+end:
+    for (i = 0; i < num; i++)
+        av_free(components[i]);
+    av_free(components);
+    return ret;
+}
+
+static av_cold int wait_for_state(OMXCodecContext *s, OMX_STATETYPE state)
+{
+    int ret = 0;
+    pthread_mutex_lock(&s->state_mutex);
+    while (s->state != state && s->error == OMX_ErrorNone)
+        pthread_cond_wait(&s->state_cond, &s->state_mutex);
+    if (s->error != OMX_ErrorNone)
+        ret = AVERROR_ENCODER_NOT_FOUND;
+    pthread_mutex_unlock(&s->state_mutex);
+    return ret;
+}
+
+static av_cold int omx_component_init(AVCodecContext *avctx, const char *role)
+{
+    OMXCodecContext *s = avctx->priv_data;
+    OMX_PARAM_COMPONENTROLETYPE role_params = { 0 };
+    OMX_PORT_PARAM_TYPE video_port_params = { 0 };
+    OMX_PARAM_PORTDEFINITIONTYPE in_port_params = { 0 }, out_port_params = { 0 };
+    OMX_VIDEO_PARAM_PORTFORMATTYPE video_port_format = { 0 };
+    OMX_VIDEO_PARAM_BITRATETYPE vid_param_bitrate = { 0 };
+    OMX_ERRORTYPE err;
+    int i, default_size;
+
+    s->version.s.nVersionMajor = 1;
+    s->version.s.nVersionMinor = 1;
+
+    err = omx_context->ptr_GetHandle(&s->handle, s->component_name, s, (OMX_CALLBACKTYPE*) &callbacks);
+    if (err != OMX_ErrorNone) {
+        av_log(avctx, AV_LOG_ERROR, "OMX_GetHandle(%s) failed: %x\n", s->component_name, err);
+        return AVERROR_UNKNOWN;
+    }
+
+    // This one crashes the mediaserver on qcom, if used over IOMX
+    INIT_STRUCT(role_params);
+    av_strlcpy(role_params.cRole, role, sizeof(role_params.cRole));
+    // Intentionally ignore errors on this one
+    OMX_SetParameter(s->handle, OMX_IndexParamStandardComponentRole, &role_params);
+
+    INIT_STRUCT(video_port_params);
+    err = OMX_GetParameter(s->handle, OMX_IndexParamVideoInit, &video_port_params);
+    CHECK(err);
+
+    s->in_port = s->out_port = -1;
+    for (i = 0; i < video_port_params.nPorts; i++) {
+        int port = video_port_params.nStartPortNumber + i;
+        OMX_PARAM_PORTDEFINITIONTYPE port_params = { 0 };
+        INIT_STRUCT(port_params);
+        port_params.nPortIndex = port;
+        err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &port_params);
+        if (err != OMX_ErrorNone) {
+            av_log(avctx, AV_LOG_WARNING, "port %d error %x\n", port, err);
+            break;
+        }
+        if (port_params.eDir == OMX_DirInput && s->in_port < 0) {
+            in_port_params = port_params;
+            s->in_port = port;
+        } else if (port_params.eDir == OMX_DirOutput && s->out_port < 0) {
+            out_port_params = port_params;
+            s->out_port = port;
+        }
+    }
+    if (s->in_port < 0 || s->out_port < 0) {
+        av_log(avctx, AV_LOG_ERROR, "No in or out port found (in %d out %d)\n", s->in_port, s->out_port);
+        return AVERROR_UNKNOWN;
+    }
+
+    INIT_STRUCT(video_port_format);
+    video_port_format.nIndex = 0;
+    video_port_format.nPortIndex = s->in_port;
+    OMX_GetParameter(s->handle, OMX_IndexParamVideoPortFormat, &video_port_format);
+    s->color_format = video_port_format.eColorFormat;
+    // We only know the actual color format here, even though we've advertised the
+    // pixel format (YUV420P) via the encoder's pix_fmts array.
+    if (s->color_format != OMX_COLOR_FormatYUV420Planar &&
+        s->color_format != OMX_COLOR_FormatYUV420PackedPlanar) {
+        av_log(avctx, AV_LOG_ERROR, "Pixel format mismatch (%d isn't supported)\n", s->color_format);
+        return AVERROR_UNKNOWN;
+    }
+
+    in_port_params.bEnabled = OMX_TRUE;
+    in_port_params.bPopulated = OMX_FALSE;
+    in_port_params.eDomain = OMX_PortDomainVideo;
+    s->num_in_buffers = in_port_params.nBufferCountActual;
+
+    in_port_params.format.video.pNativeRender = NULL;
+    in_port_params.format.video.nStride = -1;
+    in_port_params.format.video.nSliceHeight = -1;
+    in_port_params.format.video.xFramerate = 30 << 16;
+    in_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
+    in_port_params.format.video.eColorFormat = s->color_format;
+    s->stride = avctx->width;
+    s->plane_size = avctx->height;
+    in_port_params.format.video.nStride = s->stride;
+    in_port_params.format.video.nSliceHeight = s->plane_size;
+    if (avctx->framerate.den > 0 && avctx->framerate.num > 0)
+        in_port_params.format.video.xFramerate = (1 << 16) * avctx->framerate.num / avctx->framerate.den;
+    else
+        in_port_params.format.video.xFramerate = (1 << 16) * avctx->time_base.den / avctx->time_base.num;
+    in_port_params.format.video.nFrameWidth = avctx->width;
+    in_port_params.format.video.nFrameHeight = avctx->height;
+    default_size = av_image_get_buffer_size(avctx->pix_fmt, s->stride, s->plane_size, 1);
+    if (in_port_params.nBufferSize < default_size)
+        in_port_params.nBufferSize = default_size;
+
+    err = OMX_SetParameter(s->handle, OMX_IndexParamPortDefinition, &in_port_params);
+    CHECK(err);
+    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &in_port_params);
+    CHECK(err);
+
+    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
+    out_port_params.bEnabled = OMX_TRUE;
+    out_port_params.bPopulated = OMX_FALSE;
+    out_port_params.eDomain = OMX_PortDomainVideo;
+    out_port_params.format.video.pNativeRender = NULL;
+    out_port_params.format.video.nFrameWidth = avctx->width;
+    out_port_params.format.video.nFrameHeight = avctx->height;
+    out_port_params.format.video.nStride = 0;
+    out_port_params.format.video.nSliceHeight = 0;
+    out_port_params.format.video.nBitrate = avctx->bit_rate;
+    out_port_params.format.video.xFramerate = in_port_params.format.video.xFramerate;
+    out_port_params.format.video.bFlagErrorConcealment = OMX_FALSE;
+    if (avctx->codec->id == AV_CODEC_ID_MPEG4)
+        out_port_params.format.video.eCompressionFormat = OMX_VIDEO_CodingMPEG4;
+    else if (avctx->codec->id == AV_CODEC_ID_H264)
+        out_port_params.format.video.eCompressionFormat = OMX_VIDEO_CodingAVC;
+    s->num_out_buffers = out_port_params.nBufferCountActual;
+
+    err = OMX_SetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
+    CHECK(err);
+    err = OMX_GetParameter(s->handle, OMX_IndexParamPortDefinition, &out_port_params);
+    CHECK(err);
+
+    INIT_STRUCT(vid_param_bitrate);
+    vid_param_bitrate.nPortIndex = s->out_port;
+    vid_param_bitrate.eControlRate = OMX_Video_ControlRateVariable;
+    vid_param_bitrate.nTargetBitrate = avctx->bit_rate;
+    err = OMX_SetParameter(s->handle, OMX_IndexParamVideoBitrate, &vid_param_bitrate);
+    if (err != OMX_ErrorNone)
+        av_log(avctx, AV_LOG_WARNING, "Unable to set video bitrate parameter\n");
+
+    if (avctx->codec->id == AV_CODEC_ID_H264) {
+        OMX_VIDEO_PARAM_AVCTYPE avc = { 0 };
+        INIT_STRUCT(avc);
+        avc.nPortIndex = s->out_port;
+        err = OMX_GetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
+        CHECK(err);
+        avc.nBFrames = 0;
+        avc.nPFrames = avctx->gop_size - 1;
+        err = OMX_SetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
+        CHECK(err);
+    }
+
+    err = OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateIdle, NULL);
+    CHECK(err);
+
+    s->in_buffer_headers  = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
+    s->free_in_buffers    = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_in_buffers);
+    s->out_buffer_headers = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
+    s->done_out_buffers   = av_mallocz(sizeof(OMX_BUFFERHEADERTYPE*) * s->num_out_buffers);
+    if (!s->in_buffer_headers || !s->free_in_buffers || !s->out_buffer_headers || !s->done_out_buffers)
+        return AVERROR(ENOMEM);
+    for (i = 0; i < s->num_in_buffers && err == OMX_ErrorNone; i++)
+        err = OMX_AllocateBuffer(s->handle, &s->in_buffer_headers[i],  s->in_port,  s, in_port_params.nBufferSize);
+    CHECK(err);
+    s->num_in_buffers = i;
+    for (i = 0; i < s->num_out_buffers && err == OMX_ErrorNone; i++)
+        err = OMX_AllocateBuffer(s->handle, &s->out_buffer_headers[i], s->out_port, s, out_port_params.nBufferSize);
+    CHECK(err);
+    s->num_out_buffers = i;
+
+    if (wait_for_state(s, OMX_StateIdle) < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Didn't get OMX_StateIdle\n");
+        return AVERROR_UNKNOWN;
+    }
+    err = OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateExecuting, NULL);
+    CHECK(err);
+    if (wait_for_state(s, OMX_StateExecuting) < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Didn't get OMX_StateExecuting\n");
+        return AVERROR_UNKNOWN;
+    }
+
+    for (i = 0; i < s->num_out_buffers; i++)
+        OMX_FillThisBuffer(s->handle, s->out_buffer_headers[i]);
+    for (i = 0; i < s->num_in_buffers; i++)
+        s->free_in_buffers[s->num_free_in_buffers++] = s->in_buffer_headers[i];
+    return 0;
+}
+
+static av_cold void cleanup(OMXCodecContext *s)
+{
+    int i, executing;
+
+    pthread_mutex_lock(&s->state_mutex);
+    executing = s->state == OMX_StateExecuting;
+    pthread_mutex_unlock(&s->state_mutex);
+
+    if (executing) {
+        OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateIdle, NULL);
+        wait_for_state(s, OMX_StateIdle);
+        OMX_SendCommand(s->handle, OMX_CommandStateSet, OMX_StateLoaded, NULL);
+        for (i = 0; i < s->num_in_buffers; i++) {
+            OMX_BUFFERHEADERTYPE *buffer;
+            pthread_mutex_lock(&s->input_mutex);
+            while (!s->num_free_in_buffers)
+                pthread_cond_wait(&s->input_cond, &s->input_mutex);
+            buffer = s->free_in_buffers[0];
+            s->num_free_in_buffers--;
+            memmove(&s->free_in_buffers[0], &s->free_in_buffers[1], s->num_free_in_buffers * sizeof(OMX_BUFFERHEADERTYPE*));
+            pthread_mutex_unlock(&s->input_mutex);
+            OMX_FreeBuffer(s->handle, s->in_port, buffer);
+        }
+        for (i = 0; i < s->num_out_buffers; i++) {
+            OMX_BUFFERHEADERTYPE *buffer;
+            pthread_mutex_lock(&s->output_mutex);
+            while (!s->num_done_out_buffers)
+                pthread_cond_wait(&s->output_cond, &s->output_mutex);
+            buffer = s->done_out_buffers[0];
+            s->num_done_out_buffers--;
+            memmove(&s->done_out_buffers[0], &s->done_out_buffers[1], s->num_done_out_buffers * sizeof(OMX_BUFFERHEADERTYPE*));
+            pthread_mutex_unlock(&s->output_mutex);
+            OMX_FreeBuffer(s->handle, s->out_port, buffer);
+        }
+        wait_for_state(s, OMX_StateLoaded);
+    }
+    if (s->handle) {
+        omx_context->ptr_FreeHandle(s->handle);
+        s->handle = NULL;
+    }
+
+    omx_deinit();
+    pthread_cond_destroy(&s->state_cond);
+    pthread_mutex_destroy(&s->state_mutex);
+    pthread_cond_destroy(&s->input_cond);
+    pthread_mutex_destroy(&s->input_mutex);
+    pthread_cond_destroy(&s->output_cond);
+    pthread_mutex_destroy(&s->output_mutex);
+    av_freep(&s->in_buffer_headers);
+    av_freep(&s->out_buffer_headers);
+    av_freep(&s->free_in_buffers);
+    av_freep(&s->done_out_buffers);
+    av_freep(&s->output_buf);
+}
+
+static av_cold int omx_encode_init(AVCodecContext *avctx)
+{
+    OMXCodecContext *s = avctx->priv_data;
+    int ret = AVERROR_ENCODER_NOT_FOUND;
+    const char *role;
+    OMX_BUFFERHEADERTYPE *buffer;
+
+    if ((ret = omx_init(avctx, s->libname, s->libprefix)) < 0)
+        return ret;
+
+    pthread_mutex_init(&s->state_mutex, NULL);
+    pthread_cond_init(&s->state_cond, NULL);
+    pthread_mutex_init(&s->input_mutex, NULL);
+    pthread_cond_init(&s->input_cond, NULL);
+    pthread_mutex_init(&s->output_mutex, NULL);
+    pthread_cond_init(&s->output_cond, NULL);
+    s->avctx = avctx;
+    s->state = OMX_StateLoaded;
+    s->error = OMX_ErrorNone;
+
+    switch (avctx->codec->id) {
+    case AV_CODEC_ID_MPEG4:
+        role = "video_encoder.mpeg4";
+        break;
+    case AV_CODEC_ID_H264:
+        role = "video_encoder.avc";
+        break;
+    default:
+        return AVERROR(ENOSYS);
+    }
+
+    if ((ret = find_component(avctx, role, s->component_name, sizeof(s->component_name))) < 0)
+        goto fail;
+
+    av_log(avctx, AV_LOG_INFO, "Using %s\n", s->component_name);
+
+    if ((ret = omx_component_init(avctx, role)) < 0)
+        goto fail;
+
+    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+retry:
+        pthread_mutex_lock(&s->output_mutex);
+        while (!s->num_done_out_buffers)
+            pthread_cond_wait(&s->output_cond, &s->output_mutex);
+        buffer = s->done_out_buffers[0];
+        s->num_done_out_buffers--;
+        memmove(&s->done_out_buffers[0], &s->done_out_buffers[1], s->num_done_out_buffers * sizeof(OMX_BUFFERHEADERTYPE*));
+        pthread_mutex_unlock(&s->output_mutex);
+        if (buffer->nFlags & OMX_BUFFERFLAG_CODECCONFIG) {
+            if ((ret = av_reallocp(&avctx->extradata, avctx->extradata_size + buffer->nFilledLen + FF_INPUT_BUFFER_PADDING_SIZE)) < 0) {
+                avctx->extradata_size = 0;
+                goto fail;
+            }
+            memcpy(avctx->extradata + avctx->extradata_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
+            avctx->extradata_size += buffer->nFilledLen;
+            memset(avctx->extradata + avctx->extradata_size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
+        } else {
+            OMX_FillThisBuffer(s->handle, buffer);
+            goto retry;
+        }
+        OMX_FillThisBuffer(s->handle, buffer);
+        if (avctx->codec->id == AV_CODEC_ID_H264) {
+            // For H264, the extradata can be returned in two separate buffers
+            // (the videocore encoder on raspberry pi does this);
+            // therefore check that we have got both SPS and PPS before continuing.
+            int nals[32] = { 0 };
+            int i;
+            for (i = 0; i + 4 < avctx->extradata_size; i++) {
+                 if (!avctx->extradata[i + 0] &&
+                     !avctx->extradata[i + 1] &&
+                     !avctx->extradata[i + 2] &&
+                     avctx->extradata[i + 3] == 1) {
+                     nals[avctx->extradata[i + 4] & 0x1f]++;
+                 }
+            }
+            if (!nals[NAL_SPS] || !nals[NAL_PPS])
+                goto retry;
+        }
+    }
+
+    return 0;
+fail:
+    cleanup(s);
+    return ret;
+}
+
+
+static int omx_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
+                            const AVFrame *frame, int *got_packet)
+{
+    OMXCodecContext *s = avctx->priv_data;
+    int ret = 0;
+    OMX_BUFFERHEADERTYPE* buffer;
+
+    if (frame) {
+        uint8_t *dst[4];
+        int linesize[4];
+        pthread_mutex_lock(&s->input_mutex);
+        while (!s->num_free_in_buffers)
+            pthread_cond_wait(&s->input_cond, &s->input_mutex);
+        buffer = s->free_in_buffers[--s->num_free_in_buffers];
+        pthread_mutex_unlock(&s->input_mutex);
+
+        buffer->nFilledLen = av_image_fill_arrays(dst, linesize, buffer->pBuffer, avctx->pix_fmt, s->stride, s->plane_size, 1);
+        av_image_copy(dst, linesize, (const uint8_t**) frame->data, frame->linesize, avctx->pix_fmt, avctx->width, avctx->height);
+        buffer->nFlags = OMX_BUFFERFLAG_ENDOFFRAME;
+        buffer->nOffset = 0;
+        // Convert the timestamps to microseconds; some encoders can ignore
+        // the framerate and do VFR bit allocation based on timestamps.
+        buffer->nTimeStamp = to_omx_ticks(av_rescale_q(frame->pts, avctx->time_base, AV_TIME_BASE_Q));
+        OMX_EmptyThisBuffer(s->handle, buffer);
+        s->num_in_frames++;
+    }
+
+
+retry:
+    pthread_mutex_lock(&s->output_mutex);
+    if (!frame && s->num_out_frames < s->num_in_frames) {
+        while (!s->num_done_out_buffers)
+            pthread_cond_wait(&s->output_cond, &s->output_mutex);
+    }
+    if (s->num_done_out_buffers) {
+        buffer = s->done_out_buffers[0];
+        s->num_done_out_buffers--;
+        memmove(&s->done_out_buffers[0], &s->done_out_buffers[1], s->num_done_out_buffers * sizeof(OMX_BUFFERHEADERTYPE*));
+    } else {
+        buffer = NULL;
+    }
+    pthread_mutex_unlock(&s->output_mutex);
+
+    if (buffer) {
+        if (buffer->nFlags & OMX_BUFFERFLAG_CODECCONFIG && avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+            if ((ret = av_reallocp(&avctx->extradata, avctx->extradata_size + buffer->nFilledLen + FF_INPUT_BUFFER_PADDING_SIZE)) < 0) {
+                avctx->extradata_size = 0;
+                return ret;
+            }
+            memcpy(avctx->extradata + avctx->extradata_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
+            memcpy(avctx->extradata + avctx->extradata_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
+            avctx->extradata_size += buffer->nFilledLen;
+            memset(avctx->extradata + avctx->extradata_size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
+            OMX_FillThisBuffer(s->handle, buffer);
+            goto retry;
+        }
+        if (!(buffer->nFlags & OMX_BUFFERFLAG_ENDOFFRAME)) {
+            int newsize = s->output_buf_size + buffer->nFilledLen;
+            if ((ret = av_reallocp(&s->output_buf, newsize)) < 0) {
+                s->output_buf_size = 0;
+                return ret;
+            }
+            memcpy(s->output_buf + s->output_buf_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
+            s->output_buf_size += buffer->nFilledLen;
+            OMX_FillThisBuffer(s->handle, buffer);
+            goto retry;
+        }
+        s->num_out_frames++;
+        if ((ret = ff_alloc_packet(pkt, s->output_buf_size + buffer->nFilledLen)) < 0) {
+            av_log(avctx, AV_LOG_ERROR, "Error getting output packet of size %d.\n", (int)buffer->nFilledLen);
+            OMX_FillThisBuffer(s->handle, buffer);
+            return ret;
+        }
+        memcpy(pkt->data, s->output_buf, s->output_buf_size);
+        memcpy(pkt->data + s->output_buf_size, buffer->pBuffer + buffer->nOffset, buffer->nFilledLen);
+        ret = s->output_buf_size + buffer->nFilledLen;
+        av_freep(&s->output_buf);
+        s->output_buf_size = 0;
+        pkt->pts = av_rescale_q(from_omx_ticks(buffer->nTimeStamp), AV_TIME_BASE_Q, avctx->time_base);
+        // We don't currently enable b-frames for the encoders, so set
+        // pkt->dts = pkt->pts. (The calling code behaves worse if the encoder
+        // doesn't set the dts).
+        pkt->dts = pkt->pts;
+        if (buffer->nFlags & OMX_BUFFERFLAG_SYNCFRAME)
+            pkt->flags |= AV_PKT_FLAG_KEY;
+        *got_packet = 1;
+        OMX_FillThisBuffer(s->handle, buffer);
+    }
+    return ret;
+}
+
+static av_cold int omx_encode_end(AVCodecContext *avctx)
+{
+    OMXCodecContext *s = avctx->priv_data;
+
+    cleanup(s);
+    return 0;
+}
+
+#define OFFSET(x) offsetof(OMXCodecContext, x)
+#define VDE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_ENCODING_PARAM
+static const AVOption options[] = {
+    { "omx_libname", "OpenMAX library name", OFFSET(libname), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VDE },
+    { "omx_libprefix", "OpenMAX library prefix", OFFSET(libprefix), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VDE },
+    { NULL }
+};
+
+static const enum AVPixelFormat omx_encoder_pix_fmts[] = {
+    AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE
+};
+
+static const AVClass omx_mpeg4enc_class = {
+    .class_name = "OpenMAX IL MPEG4 encoder",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+AVCodec ff_mpeg4_omx_encoder = {
+    .name             = "mpeg4_omx",
+    .type             = AVMEDIA_TYPE_VIDEO,
+    .id               = AV_CODEC_ID_MPEG4,
+    .priv_data_size   = sizeof(OMXCodecContext),
+    .init             = omx_encode_init,
+    .encode2          = omx_encode_frame,
+    .close            = omx_encode_end,
+    .pix_fmts         = omx_encoder_pix_fmts,
+    .long_name        = NULL_IF_CONFIG_SMALL("OpenMAX IL MPEG4 video encoder"),
+    .capabilities     = AV_CODEC_CAP_DELAY,
+    .priv_class       = &omx_mpeg4enc_class,
+};
+
+static const AVClass omx_h264enc_class = {
+    .class_name = "OpenMAX IL H264 encoder",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+AVCodec ff_h264_omx_encoder = {
+    .name             = "h264_omx",
+    .type             = AVMEDIA_TYPE_VIDEO,
+    .id               = AV_CODEC_ID_H264,
+    .priv_data_size   = sizeof(OMXCodecContext),
+    .init             = omx_encode_init,
+    .encode2          = omx_encode_frame,
+    .close            = omx_encode_end,
+    .pix_fmts         = omx_encoder_pix_fmts,
+    .long_name        = NULL_IF_CONFIG_SMALL("OpenMAX IL H264 video encoder"),
+    .capabilities     = AV_CODEC_CAP_DELAY,
+    .priv_class       = &omx_h264enc_class,
+};