[FFmpeg-devel] [PATCH 2/2] add libyami.cpp for h264 decoding by libyami
Zhao, Halley
halley.zhao at intel.com
Wed Jan 14 07:53:18 CET 2015
Thanks for your valued comments, I followed them in updated patch set.
>> + config_buffer.profile = VAProfileNone;
>> + status = s->decoder->start(&config_buffer);
>> + if (status != DECODE_SUCCESS) {
>> + av_log(avctx, AV_LOG_ERROR, "yami h264 decoder fail to start\n");
>> + return -1;
>> + }
> Does this check the codec profile and the hw support for it etc.?
Yes, it does.
>> + if (DECODE_FORMAT_CHANGE == status) {
>> + s->format_info = s->decoder->getFormatInfo();
>> + PRINT_DECODE_THREAD("decode format change %dx%d\n",s->format_info->width,s->format_info->height);
>> + // resend the buffer
>> + status = s->decoder->decode(in_buffer);
>> + PRINT_DECODE_THREAD("decode() status=%d\n",status);
>> + avctx->width = s->format_info->width;
>> + avctx->height = s->format_info->height;
>> + avctx->pix_fmt = AV_PIX_FMT_YUV420P;
> It writes to the global AVCodecContext without any form of synchronization?
So far, libyami supports avcC formatted codec data, not byte streamed SPS/PPS in decoder->start() yet.
After byte streamed sps/pps data is supported in decoder->start(), I will move format change check to yami_init.
Do you think it will fix the issue?
>> + if (!s->decoder) // XXX, use shared pointer for s
>> + return;
>> + pthread_mutex_lock(&s->mutex_);
>> + s->decoder->renderDone(frame);
>> + pthread_mutex_unlock(&s->mutex_);
>> + av_log(avctx, AV_LOG_DEBUG, "recycle previous frame: %p\n",
>> +frame); }
> This fails if you keep a frame longer than the decoder. (Which worked with _all_ other decoders so far.)
How about keep a reference of decoder in the video frame?
>> + while (s->decode_status < DECODE_THREAD_GOT_EOS) { // we need
>> + enque eos buffer more than once
> Checking decode_status without a lock?
Yes, it seems not an issue.
We just try not to enque an empty buffer (for EOS) many times
>> + pthread_mutex_lock(&s->in_mutex);
>> + if (s->in_queue->size()<4) {
>> + s->in_queue->push_back(in_buffer);
>> + av_log(avctx, AV_LOG_VERBOSE, "wakeup decode thread ...\n");
>> + pthread_cond_signal(&s->in_cond);
...
>> + av_log(avctx, AV_LOG_DEBUG, "s->in_queue->size()=%ld, s->decode_count=%d, s->decode_count_yami=%d, too many buffer are under decoding, wait ...\n",
>> + s->in_queue->size(), s->decode_count, s->decode_count_yami);
>> + usleep(10000);
> No. Use condition variables.
I updated the patch to enque the input buffer unconditionally.
> Does it even need a decode thread?
Two threads help GPU runs in parallel
>> + do {
>> + if (!s->format_info) {
>> + usleep(10000);
> Nonsense again...
It waits during start until stream resolution/format are got, seems fine.
>> + yami_frame->fourcc = VA_FOURCC_I420;
>What about other pixel formats?
We support other formats (NV12/YUY2/RGBX etc), using GPU for color conversion automatically.
>> + if (s->decode_status == DECODE_THREAD_GOT_EOS) {
>> + usleep(10000);
>.....
After got EOS, we try to wait until an available output frame, seems fine.
>> + frame = (AVFrame*)data;
>This assignment was already done above.
Thanks, I removed it in updated patch
>> + frame->data[1] = (uint8_t*)yami_frame->pitch[0];
> Why not set linesize?
Thanks, I updated it in new patch
>> + ((AVFrame*)data)->extended_data = ((AVFrame*)data)->data;
> If you used the proper avframe functions, you wouldn't have to do this.
I don't know ffmpeg well, Which function do you mean to?
> Also, for this frame no pixel format is set.
Add it, thanks.
>> + frame->buf[0] = av_buffer_create((uint8_t*)yami_frame,
>> + sizeof(VideoFrameRawData), yami_recycle_frame, avctx, 0);
> Breaks refcounting of the YUV420P frame?
Sorry, not catch your meaning. Could you explain more?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-----Original Message-----
From: ffmpeg-devel-bounces at ffmpeg.org [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of wm4
Sent: Friday, January 09, 2015 9:45 PM
To: ffmpeg-devel at ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 2/2] add libyami.cpp for h264 decoding by libyami
On Fri, 9 Jan 2015 16:15:13 +0800
"Zhao, Halley" <halley.zhao at intel.com> wrote:
> From: "Zhao, Halley" <halley.zhao at intel.com>
>
> - do not support multi-thread decoding, it is unnecessary for hw
> - create a decode thread to interface with yami decoding, decouple
> frame in and out
> - the output frame type (raw data | drm handle | dmabuf) are specified
> in avctx->coder during init
> - yami frame is assigned to AVFrame->buf[0], yami_recycle_frame() is
> registered to AVBufferRef. then it is recycle when AVFrame/AVBufferRef
> is unref'ed.
> ---
What's the point of using this library, if ffmpeg already has most of the hw decoding infrastructure itself?
> libavcodec/libyami.cpp | 386
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 386 insertions(+)
> create mode 100644 libavcodec/libyami.cpp
>
> diff --git a/libavcodec/libyami.cpp b/libavcodec/libyami.cpp new file
> mode 100644 index 0000000..e944cde
> --- /dev/null
> +++ b/libavcodec/libyami.cpp
> @@ -0,0 +1,386 @@
> +/*
> + * libyami.cpp -- h264 decoder uses libyami
> + *
> + * Copyright (C) 2014 Intel Corporation
> + * Author: Zhao Halley<halley.zhao at intel.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 <pthread.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <deque>
> +extern "C" {
> +#include "avcodec.h"
> +#include "libavutil/imgutils.h"
> +#include "internal.h"
> +}
> +#include "VideoDecoderHost.h"
> +
> +using namespace YamiMediaCodec;
> +#ifndef VA_FOURCC_I420
> +#define VA_FOURCC_I420 VA_FOURCC('I','4','2','0') #endif #define
> +PRINT_DECODE_THREAD(format, ...) av_log(avctx, AV_LOG_VERBOSE, "##
> +decode thread ## line:%4d " format, __LINE__, ##__VA_ARGS__)
> +
> +typedef enum {
> + DECODE_THREAD_NOT_INIT = 0,
> + DECODE_THREAD_RUNING,
> + DECODE_THREAD_GOT_EOS,
> + DECODE_THREAD_EXIT,
> +} DecodeThreadStatus;
> +
> +struct YamiContext {
> + AVCodecContext *avctx;
> + pthread_mutex_t mutex_; // mutex for decoder->getOutput() and
> +YamiContext itself update (decode_status, etc)
> +
> + IVideoDecoder *decoder;
> + VideoDataMemoryType output_type;
> + const VideoFormatInfo *format_info;
> + pthread_t decode_thread_id;
> + std::deque<VideoDecodeBuffer*> *in_queue;
> + pthread_mutex_t in_mutex; // mutex for in_queue
> + pthread_cond_t in_cond; // decode thread condition wait
> + DecodeThreadStatus decode_status;
> +
> + // debug use
> + int decode_count;
> + int decode_count_yami;
> + int render_count;
> +};
> +
> +static av_cold int yami_init(AVCodecContext *avctx) {
> + YamiContext *s = (YamiContext*)avctx->priv_data;
> + Decode_Status status;
> +
> + av_log(avctx, AV_LOG_VERBOSE, "yami_init\n");
> + s->decoder = createVideoDecoder("video/h264");
> + if (!s->decoder) {
> + av_log(avctx, AV_LOG_ERROR, "fail to create libyami h264 decoder\n");
> + return -1;
> + }
> +
> + NativeDisplay native_display;
> + native_display.type = NATIVE_DISPLAY_DRM;
> + native_display.handle = 0;
> + s->decoder ->setNativeDisplay(&native_display);
> +
> + VideoConfigBuffer config_buffer;
> + memset(&config_buffer,0,sizeof(VideoConfigBuffer));
> + if (avctx->extradata && avctx->extradata_size && avctx->extradata[0] == 1) {
> + config_buffer.data = avctx->extradata;
> + config_buffer.size = avctx->extradata_size;
> + }
> + config_buffer.profile = VAProfileNone;
> + status = s->decoder->start(&config_buffer);
> + if (status != DECODE_SUCCESS) {
> + av_log(avctx, AV_LOG_ERROR, "yami h264 decoder fail to start\n");
> + return -1;
> + }
Does this check the codec profile and the hw support for it etc.?
> +
> + switch (avctx->coder_type) {
> + case 0:
> + s->output_type = VIDEO_DATA_MEMORY_TYPE_RAW_POINTER;
> + break;
> + case 1:
> + s->output_type = VIDEO_DATA_MEMORY_TYPE_DRM_NAME;
> + break;
> + case 2:
> + s->output_type = VIDEO_DATA_MEMORY_TYPE_DMA_BUF;
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "unknown output frame type: %d", avctx->coder_type);
> + break;
> + }
> +
> + s->in_queue = new std::deque<VideoDecodeBuffer*>;
> + pthread_mutex_init(&s->mutex_, NULL);
> + pthread_mutex_init(&s->in_mutex, NULL);
> + pthread_cond_init(&s->in_cond, NULL);
> + s->decode_status = DECODE_THREAD_NOT_INIT;
> + s->decode_count = 0;
> + s->decode_count_yami = 0;
> + s->render_count = 0;
> +
> + return 0;
> +}
> +
> +static void* decodeThread(void *arg)
> +{
> + AVCodecContext *avctx = (AVCodecContext*)arg;
> + YamiContext *s = (YamiContext*)avctx->priv_data;
> +
> + while (1) {
> + VideoDecodeBuffer *in_buffer = NULL;
> + // deque one input buffer
> + PRINT_DECODE_THREAD("decode thread runs one cycle start ... \n");
> + pthread_mutex_lock(&s->in_mutex);
> + if (s->in_queue->empty()) {
> + if (s->decode_status == DECODE_THREAD_GOT_EOS) {
> + pthread_mutex_unlock(&s->in_mutex);
> + break;
> + } else {
> + PRINT_DECODE_THREAD("decode thread wait because s->in_queue is empty\n");
> + pthread_cond_wait(&s->in_cond, &s->in_mutex); // wait if no todo frame is available
> + }
> + }
> +
> + if (s->in_queue->empty()) { // may wake up from EOS/Close
> + pthread_mutex_unlock(&s->in_mutex);
> + continue;
> + }
> +
> + PRINT_DECODE_THREAD("s->in_queue->size()=%ld\n", s->in_queue->size());
> + in_buffer = s->in_queue->front();
> + s->in_queue->pop_front();
> + pthread_mutex_unlock(&s->in_mutex);
> +
> + // decode one input buffer
> + PRINT_DECODE_THREAD("try to process one input buffer, in_buffer->data=%p, in_buffer->size=%d\n", in_buffer->data, in_buffer->size);
> + Decode_Status status = s->decoder->decode(in_buffer);
> + PRINT_DECODE_THREAD("decode() status=%d,
> + decode_count_yami=%d\n", status, s->decode_count_yami);
> +
> + if (DECODE_FORMAT_CHANGE == status) {
> + s->format_info = s->decoder->getFormatInfo();
> + PRINT_DECODE_THREAD("decode format change %dx%d\n",s->format_info->width,s->format_info->height);
> + // resend the buffer
> + status = s->decoder->decode(in_buffer);
> + PRINT_DECODE_THREAD("decode() status=%d\n",status);
> + avctx->width = s->format_info->width;
> + avctx->height = s->format_info->height;
> + avctx->pix_fmt = AV_PIX_FMT_YUV420P;
It writes to the global AVCodecContext without any form of synchronization?
> + }
> + s->decode_count_yami++;
> + av_free(in_buffer);
> + }
> +
> + PRINT_DECODE_THREAD("decode thread exit\n");
> + pthread_mutex_lock(&s->mutex_);
> + s->decode_status = DECODE_THREAD_EXIT;
> + pthread_mutex_unlock(&s->mutex_);
> + return NULL;
> +}
> +
> +static void yami_recycle_frame(void *opaque, uint8_t *data) {
> + AVCodecContext *avctx = (AVCodecContext*)opaque;
> + YamiContext *s = (YamiContext*)avctx->priv_data;
> + VideoFrameRawData *frame = (VideoFrameRawData*)data;
> +
> + if (!s->decoder) // XXX, use shared pointer for s
> + return;
> + pthread_mutex_lock(&s->mutex_);
> + s->decoder->renderDone(frame);
> + pthread_mutex_unlock(&s->mutex_);
> + av_log(avctx, AV_LOG_DEBUG, "recycle previous frame: %p\n",
> +frame); }
This fails if you keep a frame longer than the decoder. (Which worked with _all_ other decoders so far.)
> +
> +static int yami_decode_frame(AVCodecContext *avctx, void *data /* output frame */,
> + int *got_frame, AVPacket *avpkt
> +/* input compressed data*/) {
> + YamiContext *s = (YamiContext*)avctx->priv_data;
> + VideoDecodeBuffer *in_buffer = NULL;
> + Decode_Status status = RENDER_NO_AVAILABLE_FRAME;
> + VideoFrameRawData *yami_frame = NULL;
> + AVFrame *frame = (AVFrame*)data;
> +
> + av_log(avctx, AV_LOG_VERBOSE, "yami_decode_frame\n");
> + // append avpkt to input buffer queue
> + in_buffer = (VideoDecodeBuffer*)av_mallocz(sizeof(VideoDecodeBuffer));
> + in_buffer->data = avpkt->data;
> + in_buffer->size = avpkt->size;
> + in_buffer->timeStamp = avpkt->pts;
> + while (s->decode_status < DECODE_THREAD_GOT_EOS) { // we need
> + enque eos buffer more than once
Checking decode_status without a lock?
> + pthread_mutex_lock(&s->in_mutex);
> + if (s->in_queue->size()<4) {
> + s->in_queue->push_back(in_buffer);
> + av_log(avctx, AV_LOG_VERBOSE, "wakeup decode thread ...\n");
> + pthread_cond_signal(&s->in_cond);
> + pthread_mutex_unlock(&s->in_mutex);
> + break;
> + }
> + pthread_mutex_unlock(&s->in_mutex);
> +
> + av_log(avctx, AV_LOG_DEBUG, "s->in_queue->size()=%ld, s->decode_count=%d, s->decode_count_yami=%d, too many buffer are under decoding, wait ...\n",
> + s->in_queue->size(), s->decode_count, s->decode_count_yami);
> + usleep(10000);
No. Use condition variables.
Does it even need a decode thread?
> + };
> + s->decode_count++;
> +
> + // decode thread status update
> + pthread_mutex_lock(&s->mutex_);
> + switch (s->decode_status) {
> + case DECODE_THREAD_NOT_INIT:
> + case DECODE_THREAD_EXIT:
> + if (avpkt->data && avpkt->size) {
> + s->decode_status = DECODE_THREAD_RUNING;
> + pthread_create(&s->decode_thread_id, NULL, &decodeThread, avctx);
> + }
> + break;
> + case DECODE_THREAD_RUNING:
> + if (!avpkt->data || ! avpkt->size)
> + s->decode_status = DECODE_THREAD_GOT_EOS; // call releaseLock for seek
> + break;
> + case DECODE_THREAD_GOT_EOS:
> + s->decode_status = DECODE_THREAD_NOT_INIT;
> + break;
> + default:
> + break;
> + }
> + pthread_mutex_unlock(&s->mutex_);
> +
> + // get an output buffer from yami
> + do {
> + if (!s->format_info) {
> + usleep(10000);
Nonsense again...
> + continue;
> + }
> + yami_frame = (VideoFrameRawData*)av_malloc(sizeof(VideoFrameRawData));
> + yami_frame->memoryType = s->output_type;
> + if (s->output_type == VIDEO_DATA_MEMORY_TYPE_DRM_NAME || s->output_type == VIDEO_DATA_MEMORY_TYPE_DMA_BUF) {
> + yami_frame->fourcc = VA_FOURCC_BGRX;
> + } else {
> + yami_frame->fourcc = VA_FOURCC_I420;
What about other pixel formats?
> + }
> + yami_frame->width = s->format_info->width;
> + yami_frame->height = s->format_info->height;
> +
> + pthread_mutex_lock(&s->mutex_);
> + status = s->decoder->getOutput(yami_frame); // do not use draining flag here, both draining here and in decode thread will cause race condition
> + pthread_mutex_unlock(&s->mutex_);
> + av_log(avctx, AV_LOG_DEBUG, "getoutput() status=%d\n",status);
> + if (status == RENDER_SUCCESS)
> + break;
> +
> + if (s->decode_status == DECODE_THREAD_GOT_EOS) {
> + usleep(10000);
.....
> + continue;
> + } else {
> + *got_frame = 0;
> + return avpkt->size;
> + }
> + } while (s->decode_status == DECODE_THREAD_RUNING);
> +
> + if (status != RENDER_SUCCESS) {
> + assert(s->decode_status != DECODE_THREAD_RUNING);
> + av_log(avctx, AV_LOG_VERBOSE, "after processed EOS, return\n");
> + return avpkt->size;
> + }
> +
> + // process the output frame
> + if (s->output_type == VIDEO_DATA_MEMORY_TYPE_DRM_NAME || s->output_type == VIDEO_DATA_MEMORY_TYPE_DMA_BUF) {
> + frame = (AVFrame*)data;
This assignment was already done above.
> + frame->data[0] = (uint8_t*)yami_frame->handle;
> + frame->data[1] = (uint8_t*)yami_frame->pitch[0];
Why not set linesize?
> + ((AVFrame*)data)->extended_data = ((AVFrame*)data)->data;
If you used the proper avframe functions, you wouldn't have to do this.
Also, for this frame no pixel format is set.
> + }else {
> + AVFrame *vframe = av_frame_alloc();
> + int src_linesize[4];
> + const uint8_t *src_data[4];
> + int ret = ff_get_buffer(avctx, vframe, AV_GET_BUFFER_FLAG_REF);
> + if (ret < 0) {
> + return -1;
> + }
> +
> + src_linesize[0] = yami_frame->pitch[0];
> + src_linesize[1] = yami_frame->pitch[1];
> + src_linesize[2] = yami_frame->pitch[2];
> + uint8_t* yamidata = reinterpret_cast<uint8_t*>(yami_frame->handle);
> + src_data[0] = yamidata + yami_frame->offset[0];
> + src_data[1] = yamidata + yami_frame->offset[1];
> + src_data[2] = yamidata + yami_frame->offset[2];
> +
> + vframe->pts = yami_frame->timeStamp;
> + vframe->width = avctx->width;
> + vframe->height = avctx->height;
> + vframe->key_frame = yami_frame->flags & IS_SYNC_FRAME;
> + vframe->format = AV_PIX_FMT_YUV420P;
> + vframe->extended_data = NULL;
> + av_image_copy(vframe->data, vframe->linesize, src_data, src_linesize, avctx->pix_fmt, avctx->width, avctx->height);
> + *(AVFrame*)data = *vframe;
> + ((AVFrame*)data)->extended_data = ((AVFrame*)data)->data;
> + }
> + *got_frame = 1;
> + frame->buf[0] = av_buffer_create((uint8_t*)yami_frame,
> + sizeof(VideoFrameRawData), yami_recycle_frame, avctx, 0);
Breaks refcounting of the YUV420P frame?
> + s->render_count++;
> + assert(data->buf[0] || !*got_frame);
> + av_log(avctx, AV_LOG_VERBOSE, "decode_count_yami=%d,
> + decode_count=%d, render_count=%d\n", s->decode_count_yami,
> + s->decode_count, s->render_count);
> +
> + return avpkt->size;
> +}
> +
> +static av_cold int yami_close(AVCodecContext *avctx) {
> + YamiContext *s = (YamiContext*)avctx->priv_data;
> +
> + // wait decode thread exit
> + pthread_mutex_lock(&s->mutex_);
> + while (s->decode_status != DECODE_THREAD_EXIT) {
> + // potential race condition on s->decode_status
> + s->decode_status = DECODE_THREAD_GOT_EOS;
> + pthread_mutex_unlock(&s->mutex_);
> + pthread_cond_signal(&s->in_cond);
> + usleep(10000);
> + pthread_mutex_lock(&s->mutex_);
> + }
> + pthread_mutex_unlock(&s->mutex_);
> +
> + if (s->decoder) {
> + s->decoder->stop();
> + releaseVideoDecoder(s->decoder);
> + s->decoder = NULL;
> + }
> +
> + pthread_mutex_destroy(&s->in_mutex);
> + pthread_cond_destroy(&s->in_cond);
> + delete s->in_queue;
> + av_log(avctx, AV_LOG_VERBOSE, "yami_close\n");
> +
> + return 0;
> +}
> +
> +AVCodec ff_libyami_h264_decoder = {
> + .name = "libyami_h264",
> + .long_name = NULL_IF_CONFIG_SMALL("libyami H.264"),
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = AV_CODEC_ID_H264,
> + .capabilities = CODEC_CAP_DELAY, // it is not necessary to support multi-threads
> + .supported_framerates = NULL,
> + .pix_fmts = NULL,
> + .supported_samplerates = NULL,
> + .sample_fmts = NULL,
> + .channel_layouts = NULL,
> +#if FF_API_LOWRES
> + .max_lowres = 0,
> +#endif
> + .priv_class = NULL,
> + .profiles = NULL,
> + .priv_data_size = sizeof(YamiContext),
> + .next = NULL,
> + .init_thread_copy = NULL,
> + .update_thread_context = NULL,
> + .defaults = NULL,
> + .init_static_data = NULL,
> + .init = yami_init,
> + .encode_sub = NULL,
> + .encode2 = NULL,
> + .decode = yami_decode_frame,
> + .close = yami_close,
> + .flush = NULL, // TODO, add it
> +};
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list