[FFmpeg-devel] [PATCH] lavc/mediacodec: add hwaccel support

Matthieu Bouron matthieu.bouron at gmail.com
Fri Jul 8 16:40:14 EEST 2016


On Fri, Jul 08, 2016 at 01:21:48PM +0200, Benoit Fouet wrote:
> Hi,
> 
> On 07/07/2016 17:43, Matthieu Bouron wrote:
> 
> [...]
> 
> > 
> > 0001-lavc-add-mediacodec-hwaccel-support.patch
> > 
> > 
> >  From 9bb86990f0f7a26d25878a771f5977ae83d14769 Mon Sep 17 00:00:00 2001
> > From: Matthieu Bouron<matthieu.bouron at stupeflix.com>
> > Date: Fri, 11 Mar 2016 17:21:04 +0100
> > Subject: [PATCH] lavc: add mediacodec hwaccel support
> > 
> > ---
> >   configure                       |   1 +
> >   libavcodec/Makefile             |   6 +-
> >   libavcodec/allcodecs.c          |   1 +
> >   libavcodec/mediacodec.c         | 133 ++++++++++++++++++++
> >   libavcodec/mediacodec.h         |  88 +++++++++++++
> >   libavcodec/mediacodec_surface.c |  66 ++++++++++
> >   libavcodec/mediacodec_surface.h |  31 +++++
> >   libavcodec/mediacodec_wrapper.c |   5 +-
> >   libavcodec/mediacodecdec.c      | 270 +++++++++++++++++++++++++++++++++-------
> >   libavcodec/mediacodecdec.h      |  17 +++
> >   libavcodec/mediacodecdec_h264.c |  44 ++++++-
> >   libavcodec/version.h            |   2 +-
> >   libavutil/pixdesc.c             |   4 +
> >   libavutil/pixfmt.h              |   2 +
> >   14 files changed, 611 insertions(+), 59 deletions(-)
> >   create mode 100644 libavcodec/mediacodec.c
> >   create mode 100644 libavcodec/mediacodec.h
> >   create mode 100644 libavcodec/mediacodec_surface.c
> >   create mode 100644 libavcodec/mediacodec_surface.h
> > 
> > [...]
> > 
> > diff --git a/libavcodec/mediacodec.c b/libavcodec/mediacodec.c
> > new file mode 100644
> > index 0000000..5b79798
> > --- /dev/null
> > +++ b/libavcodec/mediacodec.c
> > @@ -0,0 +1,133 @@
> > +/*
> > + * Android MediaCodec public API functions
> > + *
> > + * Copyright (c) 2016 Matthieu Bouron <matthieu.bouron stupeflix.com>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg 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.
> > + *
> > + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#include "config.h"
> > +
> > +#if CONFIG_H264_MEDIACODEC_HWACCEL
> > +
> > +#include <jni.h>
> > +
> > +#include "libavcodec/avcodec.h"
> > +#include "libavutil/atomic.h"
> > +#include "libavutil/mem.h"
> > +
> > +#include "ffjni.h"
> > +#include "mediacodec.h"
> > +#include "mediacodecdec.h"
> > +
> > +AVMediaCodecContext *av_mediacodec_alloc_context(void)
> > +{
> > +    return av_mallocz(sizeof(AVMediaCodecContext));
> > +}
> > +
> > +int av_mediacodec_default_init(AVCodecContext *avctx, AVMediaCodecContext *ctx, void *surface)
> > +{
> > +    int ret = 0;
> > +    JNIEnv *env = NULL;
> > +    int attached = 0;
> > +
> 
> nit: env and attached don't need to be initialized

I'll keep it as is if it's OK with you as the rest of the MediaCodec
functions initialize those values. I will probably address this as a
separate patch.

> > +    env = ff_jni_attach_env(&attached, avctx);
> > +    if (!env) {
> > +        return AVERROR_EXTERNAL;
> > +    }
> > +
> > +    ctx->surface = (*env)->NewGlobalRef(env, surface);
> > +    if (ctx->surface) {
> > +        avctx->hwaccel_context = ctx;
> > +    } else {
> > +        av_log(avctx, AV_LOG_ERROR, "Could not create new global reference\n");
> > +        ret = AVERROR_EXTERNAL;
> > +    }
> > +
> > +    if (attached) {
> > +        ff_jni_detach_env(avctx);
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> > +void av_mediacodec_default_free(AVCodecContext *avctx)
> > +{
> > +    JNIEnv *env = NULL;
> > +    int attached = 0;
> > +
> 
> ditto
> 
> [...]
> 
> > diff --git a/libavcodec/mediacodec.h b/libavcodec/mediacodec.h
> > new file mode 100644
> > index 0000000..f755bd1
> > --- /dev/null
> > +++ b/libavcodec/mediacodec.h
> > @@ -0,0 +1,88 @@
> > +/*
> > + * Android MediaCodec public API
> > + *
> > + * Copyright (c) 2016 Matthieu Bouron <matthieu.bouron stupeflix.com>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg 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.
> > + *
> > + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#ifndef AVCODEC_MEDIACODEC_H
> > +#define AVCODEC_MEDIACODEC_H
> > +
> > +#include "libavcodec/avcodec.h"
> > +
> > +/**
> > + * This structure holds a reference to a android/view/Surface object that will
> > + * be used as output by the decoder.
> > + *
> > + */
> > +typedef struct AVMediaCodecContext {
> > +
> > +    /**
> > +     * android/view/Surface object reference.
> > +     */
> > +    void *surface;
> > +
> > +} AVMediaCodecContext;
> > +
> > +/**
> > + * Allocate and initialize a MediaCodec context.
> > + *
> > + * When decoding with MediaCodec is finished, the caller must free the
> > + * MediaCodec context with av_mediacodec_default_free.
> > + *
> > + * @return a pointer to a newly allocated AVMediaCodecContext on success, NULL otherwise
> > + */
> > +AVMediaCodecContext *av_mediacodec_alloc_context(void);
> > +
> > +/**
> > + * Convenience function that sets up the MediaCodec context.
> > + *
> > + * @param avctx codec context
> > + * @param ctx MediaCodec context to initialize
> > + * @param surface reference to an android/view/Surface
> > + * @return 0 on success, < 0 otherwise
> > + */
> > +int av_mediacodec_default_init(AVCodecContext *avctx, AVMediaCodecContext *ctx, void *surface);
> > +
> > +/**
> > + * This function must be called to free the MediaCodec context initialized with
> > + * av_mediacodec_default_init().
> > + *
> > + * @param avctx codec context
> > + */
> > +void av_mediacodec_default_free(AVCodecContext *avctx);
> > +
> > +/**
> > + * Opaque structure representing a MediaCodec buffer to render.
> > + */
> > +typedef struct MediaCodecBuffer AVMediaCodecBuffer;
> > +
> > +/**
> > + * Release a MediaCodec buffer and render it on the surface that is associated
> 
> nit: "render to the surface"
> same below

Fixed locally.

> 
> > + * with the decoder. This function should only be called once on a given
> > + * buffer, once released the underlying buffer returns to the codec, thus
> > + * subsequent calls to this function will have no effect.
> > + *
> > + * @param buffer the buffer to render
> > + * @param render 1 to release and render the buffer on the surface or 0 to
> > + * only release the buffer
> > + * @return 0 on success, < 0 otherwise
> > + */
> > +int av_mediacodec_release_buffer(AVMediaCodecBuffer *buffer, int render);
> > +
> > +#endif /* AVCODEC_MEDIACODEC_H */
> > diff --git a/libavcodec/mediacodec_surface.c b/libavcodec/mediacodec_surface.c
> > new file mode 100644
> > index 0000000..903ebe4
> > --- /dev/null
> > +++ b/libavcodec/mediacodec_surface.c
> > @@ -0,0 +1,66 @@
> > +/*
> > + * Android MediaCodec Surface functions
> > + *
> > + * Copyright (c) 2016 Matthieu Bouron <matthieu.bouron stupeflix.com>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg 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.
> > + *
> > + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#include <jni.h>
> > +
> > +#include "ffjni.h"
> > +#include "mediacodec_surface.h"
> > +
> > +void *ff_mediacodec_surface_ref(void *surface, void *log_ctx)
> > +{
> > +    int attached = 0;
> > +    JNIEnv *env = NULL;
> > +
> > +    void *reference = NULL;
> > +
> 
> nit: unneeded initializations
> 
> > +    env = ff_jni_attach_env(&attached, log_ctx);
> > +    if (!env) {
> > +        return NULL;
> > +    }
> > +
> > +    reference = (*env)->NewGlobalRef(env, surface);
> > +
> > +    if (attached) {
> > +        ff_jni_detach_env(log_ctx);
> > +    }
> > +
> > +    return reference;
> > +}
> > +
> > +int ff_mediacodec_surface_unref(void *surface, void *log_ctx)
> > +{
> > +    int attached = 0;
> > +    JNIEnv *env = NULL;
> > +
> 
> ditto
> 
> [...]
> 
> > diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> > index e29637e..aa5ab24 100644
> > --- a/libavcodec/mediacodecdec.c
> > +++ b/libavcodec/mediacodecdec.c
> > @@ -23,6 +23,7 @@
> >   #include <string.h>
> >   #include <sys/types.h>
> > +#include "libavutil/atomic.h"
> >   #include "libavutil/common.h"
> >   #include "libavutil/mem.h"
> >   #include "libavutil/log.h"
> > @@ -33,6 +34,8 @@
> >   #include "avcodec.h"
> >   #include "internal.h"
> > +#include "mediacodec.h"
> > +#include "mediacodec_surface.h"
> >   #include "mediacodec_sw_buffer.h"
> >   #include "mediacodec_wrapper.h"
> >   #include "mediacodecdec.h"
> > @@ -118,6 +121,10 @@ static enum AVPixelFormat mcdec_map_color_format(AVCodecContext *avctx,
> >       int i;
> >       enum AVPixelFormat ret = AV_PIX_FMT_NONE;
> > +    if (s->surface) {
> > +        return AV_PIX_FMT_MEDIACODEC;
> > +    }
> > +
> >       if (!strcmp(s->codec_name, "OMX.k3.video.decoder.avc") && color_format == COLOR_FormatYCbYCr) {
> >           s->color_format = color_format = COLOR_TI_FormatYUV420PackedSemiPlanar;
> >       }
> > @@ -134,7 +141,117 @@ static enum AVPixelFormat mcdec_map_color_format(AVCodecContext *avctx,
> >       return ret;
> >   }
> > -static int mediacodec_wrap_buffer(AVCodecContext *avctx,
> > +static void ff_mediacodec_dec_ref(MediaCodecDecContext *s)
> > +{
> > +    avpriv_atomic_int_add_and_fetch(&s->refcount, 1);
> > +}
> > +
> > +static void ff_mediacodec_dec_unref(MediaCodecDecContext *s)
> > +{
> > +    if (!s)
> > +        return;
> > +
> > +    if (!avpriv_atomic_int_add_and_fetch(&s->refcount, -1)) {
> > +        if (s->codec) {
> > +            ff_AMediaCodec_delete(s->codec);
> > +            s->codec = NULL;
> > +        }
> > +
> > +        if (s->format) {
> > +            ff_AMediaFormat_delete(s->format);
> > +            s->format = NULL;
> > +        }
> > +
> > +        if (s->surface) {
> > +            ff_mediacodec_surface_unref(s->surface, NULL);
> > +            s->surface = NULL;
> > +        }
> > +
> > +        av_freep(&s->codec_name);
> > +        av_freep(&s);
> > +    }
> > +}
> > +
> > +static void mediacodec_buffer_release(void *opaque, uint8_t *data)
> > +{
> > +    AVMediaCodecBuffer *buffer = opaque;
> > +    MediaCodecDecContext *ctx = buffer->ctx;
> > +    int released = avpriv_atomic_int_get(&buffer->released);
> > +
> > +    if (!released) {
> > +        ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, 0);
> > +    }
> > +
> > +    ff_mediacodec_dec_unref(ctx);
> > +    av_freep(&buffer);
> > +}
> > +
> > +static int mediacodec_wrap_hw_buffer(AVCodecContext *avctx,
> > +                                  MediaCodecDecContext *s,
> > +                                  ssize_t index,
> > +                                  FFAMediaCodecBufferInfo *info,
> > +                                  AVFrame *frame)
> > +{
> > +    int ret = 0;
> > +    int status = 0;
> > +    AVMediaCodecBuffer *buffer = NULL;
> > +
> > +    frame->buf[0] = NULL;
> > +    frame->width = avctx->width;
> > +    frame->height = avctx->height;
> > +    frame->format = avctx->pix_fmt;
> > +
> > +    if (avctx->pkt_timebase.num && avctx->pkt_timebase.den) {
> > +        frame->pkt_pts = av_rescale_q(info->presentationTimeUs,
> > +                                      av_make_q(1, 1000000),
> > +                                      avctx->pkt_timebase);
> > +    } else {
> > +        frame->pkt_pts = info->presentationTimeUs;
> > +    }
> > +    frame->pkt_dts = AV_NOPTS_VALUE;
> > +
> > +    buffer = av_mallocz(sizeof(AVMediaCodecBuffer));
> > +    if (!buffer) {
> > +        ret = AVERROR(ENOMEM);
> > +        goto fail;
> > +    }
> > +
> > +    buffer->released = 0;
> > +
> > +    frame->buf[0] = av_buffer_create(NULL,
> > +                                     0,
> > +                                     mediacodec_buffer_release,
> > +                                     buffer,
> > +                                     AV_BUFFER_FLAG_READONLY);
> > +
> > +    if (!frame->buf[0]) {
> > +        ret = AVERROR(ENOMEM);
> > +        goto fail;
> > +
> > +    }
> > +
> > +    buffer->ctx = s;
> > +    ff_mediacodec_dec_ref(s);
> > +
> > +    buffer->index = index;
> > +    buffer->pts = info->presentationTimeUs;
> > +
> > +    frame->data[3] = (uint8_t *)buffer;
> > +
> > +    return 0;
> > +fail:
> > +    av_freep(buffer);
> > +    av_buffer_unref(&frame->buf[0]);
> > +    status = ff_AMediaCodec_releaseOutputBuffer(s->codec, index, 0);
> > +    if (status < 0) {
> > +        av_log(avctx, AV_LOG_ERROR, "Failed to release output buffer\n");
> > +        ret = AVERROR_EXTERNAL;
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> > +static int mediacodec_wrap_sw_buffer(AVCodecContext *avctx,
> >                                     MediaCodecDecContext *s,
> >                                     uint8_t *data,
> >                                     size_t size,
> > @@ -304,6 +421,30 @@ static int mediacodec_dec_parse_format(AVCodecContext *avctx, MediaCodecDecConte
> >       return ff_set_dimensions(avctx, width, height);
> >   }
> > +
> > +static int mediacodec_dec_flush_codec(AVCodecContext *avctx, MediaCodecDecContext *s)
> > +{
> > +    FFAMediaCodec *codec = s->codec;
> > +    int status;
> > +
> > +    s->dequeued_buffer_nb = 0;
> > +
> > +    s->draining = 0;
> > +    s->flushing = 0;
> > +    s->eos = 0;
> > +
> > +    status = ff_AMediaCodec_flush(codec);
> > +    if (status < 0) {
> > +        av_log(avctx, AV_LOG_ERROR, "Failed to flush codec\n");
> > +        return AVERROR_EXTERNAL;
> > +    }
> > +
> > +    s->first_buffer = 0;
> > +    s->first_buffer_at = av_gettime();
> > +
> > +    return 0;
> > +}
> > +
> >   int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s,
> >                              const char *mime, FFAMediaFormat *format)
> >   {
> > @@ -311,7 +452,24 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s,
> >       int status;
> >       int profile;
> > +    enum AVPixelFormat pix_fmt;
> > +    enum AVPixelFormat pix_fmts[3] = {
> 
> nit: 2

Fixed locally with static const enum AVPixelFormat pix_fmts[].

> 
> > +        AV_PIX_FMT_MEDIACODEC,
> > +        AV_PIX_FMT_NONE,
> > +    };
> > +
> >       s->first_buffer_at = av_gettime();
> > +    s->refcount = 1;
> > +
> > +    pix_fmt = ff_get_format(avctx, pix_fmts);
> > +    if (pix_fmt == AV_PIX_FMT_MEDIACODEC) {
> > +        AVMediaCodecContext *user_ctx = avctx->hwaccel_context;
> > +
> > +        if (user_ctx && user_ctx->surface) {
> > +            s->surface = ff_mediacodec_surface_ref(user_ctx->surface, avctx);
> > +            av_log(avctx, AV_LOG_INFO, "Using surface %p\n", s->surface);
> > +        }
> > +    }
> >       profile = ff_AMediaCodecProfile_getProfileFromAVCodecContext(avctx);
> >       if (profile < 0) {
> > @@ -332,7 +490,7 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s,
> >           goto fail;
> >       }
> > -    status = ff_AMediaCodec_configure(s->codec, format, NULL, NULL, 0);
> > +    status = ff_AMediaCodec_configure(s->codec, format, s->surface, NULL, 0);
> >       if (status < 0) {
> >           char *desc = ff_AMediaFormat_toString(format);
> >           av_log(avctx, AV_LOG_ERROR,
> > @@ -380,7 +538,7 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> >   {
> >       int ret;
> >       int offset = 0;
> > -    int need_flushing = 0;
> > +    int need_draining = 0;
> >       uint8_t *data;
> >       ssize_t index;
> >       size_t size;
> > @@ -392,15 +550,21 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> >       int64_t input_dequeue_timeout_us = INPUT_DEQUEUE_TIMEOUT_US;
> >       int64_t output_dequeue_timeout_us = OUTPUT_DEQUEUE_TIMEOUT_US;
> > +    if (s->flushing) {
> > +        av_log(avctx, AV_LOG_ERROR, "Decoder is flushing and cannot accept new buffer "
> > +                                    "until all output buffers have been released\n");
> > +        return AVERROR_EXTERNAL;
> > +    }
> > +
> >       if (pkt->size == 0) {
> > -        need_flushing = 1;
> > +        need_draining = 1;
> >       }
> > -    if (s->flushing && s->eos) {
> > +    if (s->draining && s->eos) {
> >           return 0;
> >       }
> > -    while (offset < pkt->size || (need_flushing && !s->flushing)) {
> > +    while (offset < pkt->size || (need_draining && !s->draining)) {
> >           int size;
> >           index = ff_AMediaCodec_dequeueInputBuffer(codec, input_dequeue_timeout_us);
> > @@ -419,26 +583,37 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> >               return AVERROR_EXTERNAL;
> >           }
> > -        if (need_flushing) {
> > +        if (need_draining) {
> > +            int64_t pts = pkt->pts;
> >               uint32_t flags = ff_AMediaCodec_getBufferFlagEndOfStream(codec);
> > +            if (s->surface) {
> > +                pts = av_rescale_q(pts, avctx->pkt_timebase, av_make_q(1, 1000000));
> > +            }
> > +
> >               av_log(avctx, AV_LOG_DEBUG, "Sending End Of Stream signal\n");
> > -            status = ff_AMediaCodec_queueInputBuffer(codec, index, 0, 0, pkt->pts, flags);
> > +            status = ff_AMediaCodec_queueInputBuffer(codec, index, 0, 0, pts, flags);
> >               if (status < 0) {
> >                   av_log(avctx, AV_LOG_ERROR, "Failed to queue input empty buffer (status = %d)\n", status);
> >                   return AVERROR_EXTERNAL;
> >               }
> > -            s->flushing = 1;
> > +            s->draining = 1;
> >               break;
> >           } else {
> > +            int64_t pts = pkt->pts;
> > +
> >               size = FFMIN(pkt->size - offset, size);
> >               memcpy(data, pkt->data + offset, size);
> >               offset += size;
> > -            status = ff_AMediaCodec_queueInputBuffer(codec, index, 0, size, pkt->pts, 0);
> > +            if (s->surface && avctx->pkt_timebase.num && avctx->pkt_timebase.den) {
> > +                pts = av_rescale_q(pts, avctx->pkt_timebase, av_make_q(1, 1000000));
> > +            }
> > +
> > +            status = ff_AMediaCodec_queueInputBuffer(codec, index, 0, size, pts, 0);
> >               if (status < 0) {
> >                   av_log(avctx, AV_LOG_ERROR, "Failed to queue input buffer (status = %d)\n", status);
> >                   return AVERROR_EXTERNAL;
> > @@ -446,7 +621,7 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> >           }
> >       }
> > -    if (need_flushing || s->flushing) {
> > +    if (need_draining || s->draining) {
> >           /* If the codec is flushing or need to be flushed, block for a fair
> >            * amount of time to ensure we got a frame */
> >           output_dequeue_timeout_us = OUTPUT_DEQUEUE_BLOCK_TIMEOUT_US;
> > @@ -475,15 +650,22 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> >           }
> >           if (info.size) {
> > -            data = ff_AMediaCodec_getOutputBuffer(codec, index, &size);
> > -            if (!data) {
> > -                av_log(avctx, AV_LOG_ERROR, "Failed to get output buffer\n");
> > -                return AVERROR_EXTERNAL;
> > -            }
> > -
> > -            if ((ret = mediacodec_wrap_buffer(avctx, s, data, size, index, &info, frame)) < 0) {
> > -                av_log(avctx, AV_LOG_ERROR, "Failed to wrap MediaCodec buffer\n");
> > -                return ret;
> > +            if (s->surface) {
> > +                if ((ret = mediacodec_wrap_hw_buffer(avctx, s, index, &info, frame)) < 0) {
> > +                    av_log(avctx, AV_LOG_ERROR, "Failed to wrap MediaCodec buffer\n");
> > +                    return ret;
> > +                }
> > +            } else {
> > +                data = ff_AMediaCodec_getOutputBuffer(codec, index, &size);
> > +                if (!data) {
> > +                    av_log(avctx, AV_LOG_ERROR, "Failed to get output buffer\n");
> > +                    return AVERROR_EXTERNAL;
> > +                }
> > +
> > +                if ((ret = mediacodec_wrap_sw_buffer(avctx, s, data, size, index, &info, frame)) < 0) {
> > +                    av_log(avctx, AV_LOG_ERROR, "Failed to wrap MediaCodec buffer\n");
> > +                    return ret;
> > +                }
> >               }
> >               *got_frame = 1;
> > @@ -525,9 +707,9 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> >       } else if (ff_AMediaCodec_infoOutputBuffersChanged(codec, index)) {
> >           ff_AMediaCodec_cleanOutputBuffers(codec);
> >       } else if (ff_AMediaCodec_infoTryAgainLater(codec, index)) {
> > -        if (s->flushing) {
> > +        if (s->draining) {
> >               av_log(avctx, AV_LOG_ERROR, "Failed to dequeue output buffer within %" PRIi64 "ms "
> > -                                        "while flushing remaining frames, output will probably lack frames\n",
> > +                                        "while draining remaining frames, output will probably lack frames\n",
> >                                           output_dequeue_timeout_us / 1000);
> >           } else {
> >               av_log(avctx, AV_LOG_DEBUG, "No output buffer available, try again later\n");
> > @@ -542,39 +724,35 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> >   int ff_mediacodec_dec_flush(AVCodecContext *avctx, MediaCodecDecContext *s)
> >   {
> > -    FFAMediaCodec *codec = s->codec;
> > -    int status;
> > -
> > -    s->dequeued_buffer_nb = 0;
> > +    if (!s->surface || avpriv_atomic_int_get(&s->refcount) == 1) {
> > +        int ret;
> > -    s->flushing = 0;
> > -    s->eos = 0;
> > +        if ((ret = mediacodec_dec_flush_codec(avctx, s)) < 0) {
> > +            return ret;
> > +        }
> 
> nit:
> int ret = flush();
> if (ret < 0) ...

The rest code also uses this style, I will send a separate patch to change
it.

> 
> > -    status = ff_AMediaCodec_flush(codec);
> > -    if (status < 0) {
> > -        av_log(avctx, AV_LOG_ERROR, "Failed to flush codec\n");
> > -        return AVERROR_EXTERNAL;
> > +        return 1;
> 
> Stating that returning 1 here is to state the flush is possible could be
> cool

Fixed locally with the following comment in ff_mediacodec_dec_flush:

+        /* No frames (holding a reference to the codec) are retained by the
+         * user, thus we can flush the codec and returns accordingly */

> 
> >       }
> > -    s->first_buffer = 0;
> > -    s->first_buffer_at = av_gettime();
> > -
> > +    s->flushing = 1;
> >       return 0;
> >   }
> >   int ff_mediacodec_dec_close(AVCodecContext *avctx, MediaCodecDecContext *s)
> >   {
> > -    if (s->codec) {
> > -        ff_AMediaCodec_delete(s->codec);
> > -        s->codec = NULL;
> > -    }
> > -
> > -    if (s->format) {
> > -        ff_AMediaFormat_delete(s->format);
> > -        s->format = NULL;
> > -    }
> > -
> > -    av_freep(&s->codec_name);
> > +    ff_mediacodec_dec_unref(s);
> >       return 0;
> >   }
> > +
> > +int ff_mediacodec_dec_is_flushing(AVCodecContext *avctx, MediaCodecDecContext *s)
> > +{
> > +    return s->flushing;
> > +}
> > +
> > +AVHWAccel ff_h264_mediacodec_hwaccel = {
> > +    .name    = "mediacodec",
> > +    .type    = AVMEDIA_TYPE_VIDEO,
> > +    .id      = AV_CODEC_ID_H264,
> > +    .pix_fmt = AV_PIX_FMT_MEDIACODEC,
> > +};
> > diff --git a/libavcodec/mediacodecdec.h b/libavcodec/mediacodecdec.h
> > index 646b628..8613352 100644
> > --- a/libavcodec/mediacodecdec.h
> > +++ b/libavcodec/mediacodecdec.h
> > @@ -34,12 +34,17 @@
> >   typedef struct MediaCodecDecContext {
> > +    volatile int refcount;
> > +
> 
> I don't think this needs to be marked volatile

The avpriv_atomic_[...] functions take a volatile int *ptr. Also there are
examples in our code base that declare the atomic as volatile int (see
libavutil/buffer_internal.h, libavcodec/mmaldec.c for example). I can
be missing something though.

> 
> >       char *codec_name;
> >       FFAMediaCodec *codec;
> >       FFAMediaFormat *format;
> > +    void *surface;
> > +
> >       int started;
> > +    int draining;
> >       int flushing;
> >       int eos;
> > @@ -78,4 +83,16 @@ int ff_mediacodec_dec_flush(AVCodecContext *avctx,
> >   int ff_mediacodec_dec_close(AVCodecContext *avctx,
> >                               MediaCodecDecContext *s);
> > +int ff_mediacodec_dec_is_flushing(AVCodecContext *avctx,
> > +                                  MediaCodecDecContext *s);
> > +
> > +typedef struct MediaCodecBuffer {
> > +
> > +    MediaCodecDecContext *ctx;
> > +    ssize_t index;
> > +    int64_t pts;
> > +    volatile int released;
> > +
> 
> same here
> 
> > +} MediaCodecBuffer;
> > +
> >   #endif /* AVCODEC_MEDIACODECDEC_H */
> > diff --git a/libavcodec/mediacodecdec_h264.c b/libavcodec/mediacodecdec_h264.c
> > index 11fb677..e63943d 100644
> > --- a/libavcodec/mediacodecdec_h264.c
> > +++ b/libavcodec/mediacodecdec_h264.c
> > @@ -41,7 +41,7 @@
> >   typedef struct MediaCodecH264DecContext {
> > -    MediaCodecDecContext ctx;
> > +    MediaCodecDecContext *ctx;
> >       AVBSFContext *bsf;
> > @@ -55,7 +55,8 @@ static av_cold int mediacodec_decode_close(AVCodecContext *avctx)
> >   {
> >       MediaCodecH264DecContext *s = avctx->priv_data;
> > -    ff_mediacodec_dec_close(avctx, &s->ctx);
> > +    ff_mediacodec_dec_close(avctx, s->ctx);
> > +    s->ctx = NULL;
> >       av_fifo_free(s->fifo);
> > @@ -184,7 +185,15 @@ static av_cold int mediacodec_decode_init(AVCodecContext *avctx)
> >           goto done;
> >       }
> > -    if ((ret = ff_mediacodec_dec_init(avctx, &s->ctx, CODEC_MIME, format)) < 0) {
> > +    s->ctx = av_mallocz(sizeof(*s->ctx));
> > +    if (!s->ctx) {
> > +        av_log(avctx, AV_LOG_ERROR, "Failed to allocate MediaCodecDecContext\n");
> > +        ret = AVERROR(ENOMEM);
> > +        goto done;
> > +    }
> > +
> > +    if ((ret = ff_mediacodec_dec_init(avctx, s->ctx, CODEC_MIME, format)) < 0) {
> > +        s->ctx = NULL;
> >           goto done;
> >       }
> > @@ -233,7 +242,7 @@ static int mediacodec_process_data(AVCodecContext *avctx, AVFrame *frame,
> >   {
> >       MediaCodecH264DecContext *s = avctx->priv_data;
> > -    return ff_mediacodec_dec_decode(avctx, &s->ctx, frame, got_frame, pkt);
> > +    return ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, pkt);
> >   }
> >   static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
> > @@ -260,6 +269,29 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
> >           av_fifo_generic_write(s->fifo, &input_pkt, sizeof(input_pkt), NULL);
> >       }
> > +    /*
> > +     * MediaCodec.flush() discards both input and output buffers, thus we
> > +     * need to delay the call to this function until the user has released or
> > +     * renderered the frames he retains.
> > +     *
> > +     * After we have buffered an input packet, check if the codec is in the
> > +     * flushing state. If it is, we need to call ff_mediacodec_dec_flush.
> > +     *
> > +     * ff_mediacodec_dec_flush returns 0 if the flush cannot be performed on
> > +     * the codec (because the user retains frames). The codec stays in the
> > +     * flushing state.
> > +     *
> > +     * ff_mediacodec_dec_flush returns 1 if the flush can actually be
> > +     * performed on the codec. The codec leaves the flushing state and can
> > +     * process again packets.
> > +     *
> > +     */
> > +    if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) {
> > +        if (!ff_mediacodec_dec_flush(avctx, s->ctx)) {
> > +            return avpkt->size;
> 
> This is not describing the case when ff_mediacodec_dec_flush() returns an
> error.

Fixed locally.

> 
> > +        }
> > +    }
> > +
> >       /* process buffered data */
> >       while (!*got_frame) {
> >           /* prepare the input data -- convert to Annex B if needed */
> > @@ -271,7 +303,7 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
> >               /* no more data */
> >               if (av_fifo_size(s->fifo) < sizeof(AVPacket)) {
> >                   return avpkt->size ? avpkt->size :
> > -                    ff_mediacodec_dec_decode(avctx, &s->ctx, frame, got_frame, avpkt);
> > +                    ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, avpkt);
> >               }
> >               av_fifo_generic_read(s->fifo, &input_pkt, sizeof(input_pkt), NULL);
> > @@ -318,7 +350,7 @@ static void mediacodec_decode_flush(AVCodecContext *avctx)
> >       av_packet_unref(&s->filtered_pkt);
> > -    ff_mediacodec_dec_flush(avctx, &s->ctx);
> > +    ff_mediacodec_dec_flush(avctx, s->ctx);
> >   }
> >   AVCodec ff_h264_mediacodec_decoder = {
> > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > index 5e04754..33d4b06 100644
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -28,7 +28,7 @@
> >   #include "libavutil/version.h"
> >   #define LIBAVCODEC_VERSION_MAJOR  57
> > -#define LIBAVCODEC_VERSION_MINOR  48
> > +#define LIBAVCODEC_VERSION_MINOR  49
> >   #define LIBAVCODEC_VERSION_MICRO 102
> >   #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> > diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> > index 0dffa4d..d88aaf7 100644
> > --- a/libavutil/pixdesc.c
> > +++ b/libavutil/pixdesc.c
> > @@ -1974,6 +1974,10 @@ static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
> >           .name = "qsv",
> >           .flags = AV_PIX_FMT_FLAG_HWACCEL,
> >       },
> > +    [AV_PIX_FMT_MEDIACODEC] = {
> > +        .name = "mediacodec",
> > +        .flags = AV_PIX_FMT_FLAG_HWACCEL,
> > +    },
> >       [AV_PIX_FMT_MMAL] = {
> >           .name = "mmal",
> >           .flags = AV_PIX_FMT_FLAG_HWACCEL,
> > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> > index 0ed01c4..6f71ac0 100644
> > --- a/libavutil/pixfmt.h
> > +++ b/libavutil/pixfmt.h
> > @@ -303,6 +303,8 @@ enum AVPixelFormat {
> >       AV_PIX_FMT_GBRAP10BE,  ///< planar GBR 4:4:4:4 40bpp, big-endian
> >       AV_PIX_FMT_GBRAP10LE,  ///< planar GBR 4:4:4:4 40bpp, little-endian
> > +    AV_PIX_FMT_MEDIACODEC, ///< hardware decoding through MediaCodec
> > +
> >       AV_PIX_FMT_NB,        ///< number of pixel formats, DO NOT USE THIS if you want to link with shared libav* because the number of formats might differ between versions
> >   };
> > -- 2.9.0

Thanks for the review !

If you are OK with my comments, I will push the patch.

Matthieu
[...]


More information about the ffmpeg-devel mailing list