[PATCH 0/2] Add h264 decoder (hw acceleration) through libyami
From: "Zhao, Halley" <halley.zhao@intel.com> libyami is a core codec library to support hw acceleration basing on vaapi: https://github.com/01org/libyami this patch add h264dec basing on libyami, similar to the solution basing on libstagefright. moreover, it supports dma_buf and other frame mode, enables texture video easily. here is an example player for texture video rendering uses ffmpeg/yami: https://github.com/01org/player-ffmpeg-yami Zhao, Halley (2): update configure etc to use libyami for h264 decode add libyami.cpp for h264 decoding by libyami configure | 24 ++- libavcodec/Makefile | 1 + libavcodec/allcodecs.c | 1 + libavcodec/libyami.cpp | 386 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 407 insertions(+), 5 deletions(-) mode change 100644 => 100755 libavcodec/Makefile create mode 100644 libavcodec/libyami.cpp -- 1.8.3.2
From: "Zhao, Halley" <halley.zhao@intel.com> --- configure | 24 +++++++++++++++++++----- libavcodec/Makefile | 1 + libavcodec/allcodecs.c | 1 + 3 files changed, 21 insertions(+), 5 deletions(-) mode change 100644 => 100755 libavcodec/Makefile diff --git a/configure b/configure index d4a86c0..9c54ffc 100755 --- a/configure +++ b/configure @@ -236,6 +236,7 @@ External library support: --enable-libspeex enable Speex de/encoding via libspeex [no] --enable-libssh enable SFTP protocol via libssh [no] --enable-libstagefright-h264 enable H.264 decoding via libstagefright [no] + --enable-libyami-h264 enable H.264 decoding via libyami [no] --enable-libtheora enable Theora encoding via libtheora [no] --enable-libtwolame enable MP2 encoding via libtwolame [no] --enable-libutvideo enable Ut Video encoding and decoding via libutvideo [no] @@ -1031,7 +1032,8 @@ check_func_headers(){ echo "long check_$func(void) { return (long) $func; }" done echo "int main(void) { return 0; }" - } | check_ld "cc" "$@" && enable $funcs && enable_safe $headers + ## both C++/C header check goes here, use 'cxx' + } | check_ld "cxx" "$@" && enable $funcs && enable_safe $headers } check_class_headers_cpp(){ @@ -1092,14 +1094,15 @@ check_lib_cpp(){ check_pkg_config(){ log check_pkg_config "$@" - pkgandversion="$1" + ## one component may depend on multiple pkgs + allpkgs="$1" pkg="${1%% *}" headers="$2" funcs="$3" shift 3 - check_cmd $pkg_config --exists --print-errors $pkgandversion || return - pkg_cflags=$($pkg_config --cflags $pkg_config_flags $pkg) - pkg_libs=$($pkg_config --libs $pkg_config_flags $pkg) + check_cmd $pkg_config --exists --print-errors $allpkgs || return + pkg_cflags=$($pkg_config --cflags $pkg_config_flags $allpkgs) + pkg_libs=$($pkg_config --libs $pkg_config_flags $allpkgs) check_func_headers "$headers" "$funcs" $pkg_cflags $pkg_libs "$@" && set_safe ${pkg}_cflags $pkg_cflags && set_safe ${pkg}_libs $pkg_libs @@ -1210,6 +1213,14 @@ require_pkg_config(){ add_extralibs $(get_safe ${pkg}_libs) } +cxx_extralibs_added="no" +add_cxx_extralibs(){ + if [ $cxx_extralibs_added = "no"]; then + add_extralibs "-lstdc++ -Wl,--no-as-needed -ldl" + cxx_extralibs_added="yes" + fi +} + require_libfreetype(){ log require_libfreetype "$@" pkg="freetype2" @@ -1371,6 +1382,7 @@ EXTERNAL_LIBRARY_LIST=" libspeex libssh libstagefright_h264 + libyami_h264 libtheora libtwolame libutvideo @@ -2368,6 +2380,7 @@ libspeex_decoder_deps="libspeex" libspeex_encoder_deps="libspeex" libspeex_encoder_select="audio_frame_queue" libstagefright_h264_decoder_deps="libstagefright_h264" +libyami_h264_decoder_deps="libyami_h264" libtheora_encoder_deps="libtheora" libtwolame_encoder_deps="libtwolame" libvo_aacenc_encoder_deps="libvo_aacenc" @@ -4884,6 +4897,7 @@ enabled libspeex && require_pkg_config speex speex/speex.h speex_decode enabled libstagefright_h264 && require_cpp libstagefright_h264 "binder/ProcessState.h media/stagefright/MetaData.h media/stagefright/MediaBufferGroup.h media/stagefright/MediaDebug.h media/stagefright/MediaDefs.h media/stagefright/OMXClient.h media/stagefright/OMXCodec.h" android::OMXClient -lstagefright -lmedia -lutils -lbinder -lgnustl_static +enabled libyami_h264 && require_pkg_config "libyami_decoder libva" VideoDecoderHost.h createVideoDecoder && add_cxx_extralibs() enabled libtheora && require libtheora theora/theoraenc.h th_info_init -ltheoraenc -ltheoradec -logg enabled libtwolame && require libtwolame twolame.h twolame_init -ltwolame && { check_lib twolame.h twolame_encode_buffer_float32_interleaved -ltwolame || diff --git a/libavcodec/Makefile b/libavcodec/Makefile old mode 100644 new mode 100755 index fa0f53d..d8697ff --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -747,6 +747,7 @@ OBJS-$(CONFIG_LIBSHINE_ENCODER) += libshine.o OBJS-$(CONFIG_LIBSPEEX_DECODER) += libspeexdec.o OBJS-$(CONFIG_LIBSPEEX_ENCODER) += libspeexenc.o OBJS-$(CONFIG_LIBSTAGEFRIGHT_H264_DECODER)+= libstagefright.o +OBJS-$(CONFIG_LIBYAMI_H264_DECODER) += libyami.o OBJS-$(CONFIG_LIBTHEORA_ENCODER) += libtheoraenc.o OBJS-$(CONFIG_LIBTWOLAME_ENCODER) += libtwolame.o OBJS-$(CONFIG_LIBUTVIDEO_DECODER) += libutvideodec.o diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index 0d39d33..beccc74 100644 --- a/libavcodec/allcodecs.c +++ b/libavcodec/allcodecs.c @@ -97,6 +97,7 @@ void avcodec_register_all(void) REGISTER_HWACCEL(WMV3_VDPAU, wmv3_vdpau); /* video codecs */ + REGISTER_DECODER(LIBYAMI_H264, libyami_h264); REGISTER_ENCODER(A64MULTI, a64multi); REGISTER_ENCODER(A64MULTI5, a64multi5); REGISTER_DECODER(AASC, aasc); -- 1.8.3.2
Hi, On Fri, Jan 9, 2015 at 3:15 AM, Zhao, Halley <halley.zhao@intel.com> wrote:
@@ -1031,7 +1032,8 @@ check_func_headers(){ echo "long check_$func(void) { return (long) $func; }" done echo "int main(void) { return 0; }" - } | check_ld "cc" "$@" && enable $funcs && enable_safe $headers + ## both C++/C header check goes here, use 'cxx' + } | check_ld "cxx" "$@" && enable $funcs && enable_safe $headers }
This seems wrong, as in, this will likely use c++ even when it's not necessary. Why not create a new macro check_func_headers_cxx? Ronald
On Fri, Jan 09, 2015 at 04:15:12PM +0800, Zhao, Halley wrote:
From: "Zhao, Halley" <halley.zhao@intel.com>
--- configure | 24 +++++++++++++++++++----- libavcodec/Makefile | 1 + libavcodec/allcodecs.c | 1 + 3 files changed, 21 insertions(+), 5 deletions(-) mode change 100644 => 100755 libavcodec/Makefile
this breaks configure after this patch pkg_config just fails like in ERROR: libass not found using pkg-config while it works fine before [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire
From: "Zhao, Halley" <halley.zhao@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. --- 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@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; + } + + 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; + } + 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); +} + +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 + 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); + }; + 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); + 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; + } + 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; + frame->data[0] = (uint8_t*)yami_frame->handle; + frame->data[1] = (uint8_t*)yami_frame->pitch[0]; + ((AVFrame*)data)->extended_data = ((AVFrame*)data)->data; + }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); + 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 +}; -- 1.8.3.2
On Fri, 9 Jan 2015 16:15:13 +0800 "Zhao, Halley" <halley.zhao@intel.com> wrote:
From: "Zhao, Halley" <halley.zhao@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@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 +};
On Fri, 9 Jan 2015 14:44:32 +0100 wm4 <nfxjfg@googlemail.com> wrote:
What's the point of using this library, if ffmpeg already has most of the hw decoding infrastructure itself?
probably so we dont have to maintain every single little hw decoding and encoding feature , and libyami will do this for us. same thing libstagefright does for us. so we dont have to test our ffmpeg hw decoders against every single piece of hardware. the future plans for libyami are pretty nice, https://www.mail-archive.com/libva@lists.freedesktop.org/msg02826.html -compn
On Fri, 9 Jan 2015 17:17:00 -0500 compn <tempn@mi.rr.com> wrote:
On Fri, 9 Jan 2015 14:44:32 +0100 wm4 <nfxjfg@googlemail.com> wrote:
What's the point of using this library, if ffmpeg already has most of the hw decoding infrastructure itself?
probably so we dont have to maintain every single little hw decoding and encoding feature , and libyami will do this for us.
Maintaining decoders is the point of this project. Besides, there are some more things missing from the patch.
same thing libstagefright does for us. so we dont have to test our ffmpeg hw decoders against every single piece of hardware.
The libstagefright decoding wrapper is apparently a piece of shit. Who even uses it?
the future plans for libyami are pretty nice, https://www.mail-archive.com/libva@lists.freedesktop.org/msg02826.html
I don't see any nice things.
Hi, On Fri, Jan 9, 2015 at 5:30 PM, wm4 <nfxjfg@googlemail.com> wrote:
On Fri, 9 Jan 2015 17:17:00 -0500 compn <tempn@mi.rr.com> wrote:
On Fri, 9 Jan 2015 14:44:32 +0100 wm4 <nfxjfg@googlemail.com> wrote:
What's the point of using this library, if ffmpeg already has most of the hw decoding infrastructure itself?
probably so we dont have to maintain every single little hw decoding and encoding feature , and libyami will do this for us.
Maintaining decoders is the point of this project.
Yeah, I also share this same concern. What's next, a gstreamer wrapper? Ronald
I understand you concern. The wrapper doesn't help much to ffmpeg itself, however, it benefits much to the apps uses ffmpeg, to pick up hw capability for codec. -----Original Message----- From: ffmpeg-devel-bounces@ffmpeg.org [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Ronald S. Bultje Sent: Saturday, January 10, 2015 9:49 AM To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH 2/2] add libyami.cpp for h264 decoding by libyami Hi, On Fri, Jan 9, 2015 at 5:30 PM, wm4 <nfxjfg@googlemail.com> wrote:
On Fri, 9 Jan 2015 17:17:00 -0500 compn <tempn@mi.rr.com> wrote:
On Fri, 9 Jan 2015 14:44:32 +0100 wm4 <nfxjfg@googlemail.com> wrote:
What's the point of using this library, if ffmpeg already has most of the hw decoding infrastructure itself?
probably so we dont have to maintain every single little hw decoding and encoding feature , and libyami will do this for us.
Maintaining decoders is the point of this project.
Yeah, I also share this same concern. What's next, a gstreamer wrapper? Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Hi, On Mon, Jan 12, 2015 at 12:59 AM, Zhao, Halley <halley.zhao@intel.com> wrote:
I understand you concern. The wrapper doesn't help much to ffmpeg itself, however, it benefits much to the apps uses ffmpeg, to pick up hw capability for codec.
So specifically, what are the features that you get with yaml that ffmpeg doesn't already provide (that is: yaml is itself just a wrapper for other stuff; what does it wrap that ffmpeg doesn't have already?). I see h264enc/vp8dec/vp8enc/vp9dec/jpegdec/jpegenc vaapi. Anything else? Why not just implement these in ffmpeg directly? (Note that indeed, we did not link to libvpx to get vp9/vp8 decoding; we wrote our own, and it worked _much_ better. That same may be true here also.) Ronald
On Mon, 12 Jan 2015 08:24:35 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Hi,
On Mon, Jan 12, 2015 at 12:59 AM, Zhao, Halley <halley.zhao@intel.com> wrote:
I understand you concern. The wrapper doesn't help much to ffmpeg itself, however, it benefits much to the apps uses ffmpeg, to pick up hw capability for codec.
So specifically, what are the features that you get with yaml that ffmpeg doesn't already provide (that is: yaml is itself just a wrapper for other stuff; what does it wrap that ffmpeg doesn't have already?). I see h264enc/vp8dec/vp8enc/vp9dec/jpegdec/jpegenc vaapi. Anything else? Why not just implement these in ffmpeg directly?
From what I can see: - libyami doesn't require the relatively terrible boilerplate for hwaccel (creating the hw decoder yourself, maintaining a surface pool, etc.), making this lib very attractive for those who want something simple, and who possibly don't even want to depend on ffmpeg - uses some new APIs (not supported by all drivers yet), which work around vaapi suckage, such as requiring a shared context. Namely, the dma_buf stuff. I wonder how well the latter actually works...
(Note that indeed, we did not link to libvpx to get vp9/vp8 decoding; we wrote our own, and it worked _much_ better. That same may be true here also.)
Hi, On Mon, Jan 12, 2015 at 9:04 AM, wm4 <nfxjfg@googlemail.com> wrote:
On Mon, 12 Jan 2015 08:24:35 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Hi,
On Mon, Jan 12, 2015 at 12:59 AM, Zhao, Halley <halley.zhao@intel.com> wrote:
I understand you concern. The wrapper doesn't help much to ffmpeg itself, however, it benefits much to the apps uses ffmpeg, to pick up hw capability for codec.
So specifically, what are the features that you get with yaml that ffmpeg doesn't already provide (that is: yaml is itself just a wrapper for other stuff; what does it wrap that ffmpeg doesn't have already?). I see h264enc/vp8dec/vp8enc/vp9dec/jpegdec/jpegenc vaapi. Anything else? Why not just implement these in ffmpeg directly?
From what I can see:
- libyami doesn't require the relatively terrible boilerplate for hwaccel (creating the hw decoder yourself, maintaining a surface pool, etc.), making this lib very attractive for those who want something simple, and who possibly don't even want to depend on ffmpeg - uses some new APIs (not supported by all drivers yet), which work around vaapi suckage, such as requiring a shared context. Namely, the dma_buf stuff.
I wonder how well the latter actually works...
So I don't know if anyone remembers me ranting about libxyz suckage, but yami sounds a lot like xyz. So: what are the chances that five years down the line, this lib ends up being unmaintained, having security issues, portability issues etc., and us basically just being stuck with yet another xyz that doesn't really solve our issue so we have to re-do it internally anyway a few years down the line, wishing we had done so a few years earlier? There's examples of good xyzs, e.g. x264 is a most wonderful software project; there's also examples of (imo) fails, such as librtmp. What guarantees do we have that yami is a good xyz? Ronald
Thanks your explanation. 1. I know our team/Intel plan to maintain this lib for the following years. I'm a developer, I can't promise more. 2. as to libxyz, it is some way like investment: Nobody is sure of the future, but make decision basing on investment and profit. A small yami wrapper enables rich codec on Intel platform, I think it deserve to try. Moreover, you can disable it easily if you don't like it. -----Original Message----- From: ffmpeg-devel-bounces@ffmpeg.org [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Ronald S. Bultje Sent: Monday, January 12, 2015 10:35 PM To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH 2/2] add libyami.cpp for h264 decoding by libyami Hi, On Mon, Jan 12, 2015 at 9:04 AM, wm4 <nfxjfg@googlemail.com> wrote:
On Mon, 12 Jan 2015 08:24:35 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Hi,
On Mon, Jan 12, 2015 at 12:59 AM, Zhao, Halley <halley.zhao@intel.com> wrote:
I understand you concern. The wrapper doesn't help much to ffmpeg itself, however, it benefits much to the apps uses ffmpeg, to pick up hw capability for codec.
So specifically, what are the features that you get with yaml that ffmpeg doesn't already provide (that is: yaml is itself just a wrapper for other stuff; what does it wrap that ffmpeg doesn't have already?). I see h264enc/vp8dec/vp8enc/vp9dec/jpegdec/jpegenc vaapi. Anything else? Why not just implement these in ffmpeg directly?
From what I can see:
- libyami doesn't require the relatively terrible boilerplate for hwaccel (creating the hw decoder yourself, maintaining a surface pool, etc.), making this lib very attractive for those who want something simple, and who possibly don't even want to depend on ffmpeg - uses some new APIs (not supported by all drivers yet), which work around vaapi suckage, such as requiring a shared context. Namely, the dma_buf stuff.
I wonder how well the latter actually works...
So I don't know if anyone remembers me ranting about libxyz suckage, but yami sounds a lot like xyz. So: what are the chances that five years down the line, this lib ends up being unmaintained, having security issues, portability issues etc., and us basically just being stuck with yet another xyz that doesn't really solve our issue so we have to re-do it internally anyway a few years down the line, wishing we had done so a few years earlier? There's examples of good xyzs, e.g. x264 is a most wonderful software project; there's also examples of (imo) fails, such as librtmp. What guarantees do we have that yami is a good xyz? Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Wed, Jan 14, 2015 at 01:35:14AM +0000, Zhao, Halley wrote:
Thanks your explanation. 1. I know our team/Intel plan to maintain this lib for the following years. I'm a developer, I can't promise more.
will someone also maintain the libyami wraper code from this patch ? If so the patch should add that person to the MAINTAINERs file [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates
On 14 Jan, Zhao, Halley wrote :
1. I know our team/Intel plan to maintain this lib for the following years. I'm a developer, I can't promise more.
Many people have made such claims and backtracked in the end.
2. as to libxyz, it is some way like investment: Nobody is sure of the future, but make decision basing on investment and profit.
A small yami wrapper enables rich codec on Intel platform, I think it deserve to try. Moreover, you can disable it easily if you don't like it.
Sorry, but what does it add to FFmpeg, exactly? "Rich codec" means nothing... FFmpeg has the best software codec library and supports already both VAAPI and VDPAU, on Linux. This smells a lot like NIH, bad design and feature creep... -- Jean-Baptiste Kempf http://www.jbkempf.com/ - +33 672 704 734 Sent from my Electronic Device
On Mon, 12 Jan 2015 08:24:35 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Anything else? Why not just implement these in ffmpeg directly?
because (i'm guessing) they are developing libyami with other projects, not ffmpeg. i know everyone wants to reinvent the wheels here, but also that may have a detrimental effect on development if nothing gets in except ffmpeg-style-wheels. i'm not sure how useful the codec wrapper will be, but it does have features we dont yet. if no one wants these libs , i'm not going to push the issue. -compn
On Mon, 12 Jan 2015 10:07:56 -0500 compn <tempn@mi.rr.com> wrote:
On Mon, 12 Jan 2015 08:24:35 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Anything else? Why not just implement these in ffmpeg directly?
because (i'm guessing) they are developing libyami with other projects, not ffmpeg.
i know everyone wants to reinvent the wheels here, but also that may have a detrimental effect on development if nothing gets in except ffmpeg-style-wheels.
i'm not sure how useful the codec wrapper will be, but it does have features we dont yet. if no one wants these libs , i'm not going to push the issue.
How about the fact that libyami reinvented the wheel long after ffmpeg? Look at all those codec parsers they have. There are many things that seem to stink about this lib. Having a C++ API is my least favorite one. So I guess I can say that in my opinion, the ffmpeg native support should be improved, instead of duplicating functionality all over the place.
Hi, On Mon, Jan 12, 2015 at 10:07 AM, compn <tempn@mi.rr.com> wrote:
On Mon, 12 Jan 2015 08:24:35 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Anything else? Why not just implement these in ffmpeg directly?
because (i'm guessing) they are developing libyami with other projects, not ffmpeg.
i know everyone wants to reinvent the wheels here, but also that may have a detrimental effect on development if nothing gets in except ffmpeg-style-wheels.
These are two extremes: every single C function should be shareable between any two random software projects, or we should have no dependencies at all. In the real world, we balance these extremes by the practical implications (benefits vs. risks) associated with it. - too big dependency tree (how long does it take you to build gnome? I know users don't care, but what if you're a dev? I'm a dev) - are talented developers likely to maintain the alternate codebase (as opposed to a hypothetical impl in our tree)? - are talented (app) developers likely to use ffmpeg if we use a lib's implementation, rather than our own? - will they be around in 5 years? will they fix bugs? security issues? portability issues? will they care about ffmpeg-specific exposed bugs in their code? vs. - how long will it take us to implement these features ourselves? - are we the right people to take on long-term maintenance of such a feature? - are talented (app) developers interested in such features likely to use it if we directly provide it, as opposed to through some other lib (which is linked into ffmpeg)? - are talented developers likely to help us maintain the feature in our codebase as opposed to the impl in another lib? It's very complex. I don't have straight answers to all of these. Ronald
On 12 Jan, Ronald S. Bultje wrote :
- how long will it take us to implement these features ourselves?
What features? -- Jean-Baptiste Kempf http://www.jbkempf.com/ - +33 672 704 734 Sent from my Electronic Device
On Wed, 14 Jan 2015 04:35:26 +0100 Jean-Baptiste Kempf <jb@videolan.org> wrote:
On 12 Jan, Ronald S. Bultje wrote :
- how long will it take us to implement these features ourselves?
What features?
Most importantly, it requires less boilerplate by the user. (Somewhat similar to "hwaccel 2.0" Libav is dreaming about.) Though it achieves that by using newer kernel/driver mechanisms (dma_buf), instead of requiring the user to use vaapi directly.
Hi, On Tue, Jan 13, 2015 at 10:35 PM, Jean-Baptiste Kempf <jb@videolan.org> wrote:
On 12 Jan, Ronald S. Bultje wrote :
- how long will it take us to implement these features ourselves?
What features?
You know, J-B, you're one of the very few that can see over this salesperson talk of "rich" and "very" to actually get to relevant content. Thank you. He specifically mentioned that yami supports (in addition to things ffmpeg already does) hardware h264 encoding, hardware vp8 decoding/decoding, hardware vp9 decoding and hardware jpeg encoding/decoding (all based on vaapi as underlying api). Ronald
On 14 Jan, Ronald S. Bultje wrote :
He specifically mentioned that yami supports (in addition to things ffmpeg already does) hardware h264 encoding, hardware vp8 decoding/decoding, hardware vp9 decoding and hardware jpeg encoding/decoding (all based on vaapi as underlying api).
Well, first, I really doubt that the Intel chips actually have VP8 and VP9 encoding in hardware and not in an software driver. Actually I'm quite sure they don't. Then, adding 3 encoders to the VAAPI code directly in FFmpeg seems way better than adding yet another library, that will be unmaintained, buggy and riddled with security issues. I don't understand this new fetichism of adding external dependencies to FFmpeg, because we can. With my kindest regards, -- Jean-Baptiste Kempf http://www.jbkempf.com/ - +33 672 704 734 Sent from my Electronic Device
Hi, On Wed, Jan 14, 2015 at 1:53 PM, Jean-Baptiste Kempf <jb@videolan.org> wrote:
On 14 Jan, Ronald S. Bultje wrote :
He specifically mentioned that yami supports (in addition to things ffmpeg already does) hardware h264 encoding, hardware vp8 decoding/decoding, hardware vp9 decoding and hardware jpeg encoding/decoding (all based on vaapi as underlying api).
Well, first, I really doubt that the Intel chips actually have VP8 and VP9 encoding in hardware and not in an software driver. Actually I'm quite sure they don't.
Then, adding 3 encoders to the VAAPI code directly in FFmpeg seems way better than adding yet another library, that will be unmaintained, buggy and riddled with security issues.
I don't understand this new fetichism of adding external dependencies to FFmpeg, because we can.
So, I do partially agree. I guess the sour reality is, though: is anyone on this mailinglist interested in and capable of writing e.g. a native vaapi jpeg decoder (or h264 encoder, or ...)? FFmpeg sometimes does suffer from feature creep which doesn't help the overall project's health or code quality (hi libmpcodecs!). Ronald
On Wed, Jan 14, 2015 at 07:53:03PM +0100, Jean-Baptiste Kempf wrote:
On 14 Jan, Ronald S. Bultje wrote :
He specifically mentioned that yami supports (in addition to things ffmpeg already does) hardware h264 encoding, hardware vp8 decoding/decoding, hardware vp9 decoding and hardware jpeg encoding/decoding (all based on vaapi as underlying api).
Well, first, I really doubt that the Intel chips actually have VP8 and VP9 encoding in hardware and not in an software driver. Actually I'm quite sure they don't.
Then, adding 3 encoders to the VAAPI code directly in FFmpeg seems way better than adding yet another library, that will be unmaintained, buggy and riddled with security issues.
i agree its better if there is a volunteer who does implement it but this hangs on that "if"
I don't understand this new fetichism of adding external dependencies to FFmpeg, because we can.
With my kindest regards,
-- Jean-Baptiste Kempf http://www.jbkempf.com/ - +33 672 704 734 Sent from my Electronic Device _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire
On Mon, 12 Jan 2015 10:30:34 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Hi,
On Mon, Jan 12, 2015 at 10:07 AM, compn <tempn@mi.rr.com> wrote:
On Mon, 12 Jan 2015 08:24:35 -0500 "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
Anything else? Why not just implement these in ffmpeg directly?
because (i'm guessing) they are developing libyami with other projects, not ffmpeg.
i know everyone wants to reinvent the wheels here, but also that may have a detrimental effect on development if nothing gets in except ffmpeg-style-wheels.
These are two extremes: every single C function should be shareable between any two random software projects, or we should have no dependencies at all.
In the real world, we balance these extremes by the practical implications (benefits vs. risks) associated with it. - too big dependency tree (how long does it take you to build gnome? I know users don't care, but what if you're a dev? I'm a dev) - are talented developers likely to maintain the alternate codebase (as opposed to a hypothetical impl in our tree)? - are talented (app) developers likely to use ffmpeg if we use a lib's implementation, rather than our own? - will they be around in 5 years? will they fix bugs? security issues? portability issues? will they care about ffmpeg-specific exposed bugs in their code?
vs.
- how long will it take us to implement these features ourselves? - are we the right people to take on long-term maintenance of such a feature? - are talented (app) developers interested in such features likely to use it if we directly provide it, as opposed to through some other lib (which is linked into ffmpeg)? - are talented developers likely to help us maintain the feature in our codebase as opposed to the impl in another lib?
It's very complex. I don't have straight answers to all of these.
thank you, this is exactly what i was thinking and there is no easy answer to any of these questions. i really appreciate the long writeup. i say if intel is going to maintain it, go for it. if we are going to reimplement all this into vaapi ourselves, we need a test lib for it anyway (also to steal from although its c++). i think whats going to happen is either we get a bunch more new api (how much longer do you think vdpau api will be used?), or a bunch more new features in those old api (encoding and decoding of a ton of formats/codecs and extendable in the future for future codecs like h265). which still means a new api change in vaapi/vdpau with old cards not supporting xx feature and new cards supporting it. imo theres no way we can maintain all hardware and all features ourselves. how far are we on a hwaccel fate test ? we cant even test the features we have now automatically! we can either convince devels who are working on other libs to do it The FFmpeg Way™ and we review and help those devels, or we go along with whatever lib wrapper and move on. the bikeshed being 'we are tired of more api that might or will go unmaintained' vs 'we just want it to work for a few years until the next thing comes along'. i'm not the one who has to maintain it. if the underlying lib being unmaintained in the future is an issue, how much of an issue is it when theres only one or two active devs with the hardware required? i wonder how vlc does its hwaccel testing? maybe we should steal from them. -compn
Maintaining decoders is the point of this project. Yes, I agree the core of ffmpeg is codec; But, from the user, ffmpeg is usually treated as a light-weight media framework. Being a media framework, it is good to leverage hw codec in many cases.
Besides, there are some more things missing from the patch. What's the missing? I'm glad to hear it and to fix it.
The libstagefright decoding wrapper is apparently a piece of shit. Who even uses it? I think libstagefright wrapper is good by giving more options to application, you dislike it, but other may do. The bad of it is on buffer sharing; we solve it by dma_buf.
I don't see any nice things. The example player demonstrates how the AVFrame interface will be improved with dma_buf. I do think dma_buf is a good buffer sharing mechanism to promote hw codec. https://github.com/01org/player-ffmpeg-yami
-----Original Message----- From: ffmpeg-devel-bounces@ffmpeg.org [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of wm4 Sent: Saturday, January 10, 2015 6:30 AM To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH 2/2] add libyami.cpp for h264 decoding by libyami On Fri, 9 Jan 2015 17:17:00 -0500 compn <tempn@mi.rr.com> wrote:
On Fri, 9 Jan 2015 14:44:32 +0100 wm4 <nfxjfg@googlemail.com> wrote:
What's the point of using this library, if ffmpeg already has most of the hw decoding infrastructure itself?
probably so we dont have to maintain every single little hw decoding and encoding feature , and libyami will do this for us.
Maintaining decoders is the point of this project. Besides, there are some more things missing from the patch.
same thing libstagefright does for us. so we dont have to test our ffmpeg hw decoders against every single piece of hardware.
The libstagefright decoding wrapper is apparently a piece of shit. Who even uses it?
the future plans for libyami are pretty nice, https://www.mail-archive.com/libva@lists.freedesktop.org/msg02826.html
I don't see any nice things. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Mon, 12 Jan 2015 00:46:26 +0000 "Zhao, Halley" <halley.zhao@intel.com> wrote:
Maintaining decoders is the point of this project. Yes, I agree the core of ffmpeg is codec; But, from the user, ffmpeg is usually treated as a light-weight media framework. Being a media framework, it is good to leverage hw codec in many cases.
There's already vaapi support in ffmpeg though. While I'm not opposed to improving the hw decoding situation in general. I think it'd be better to improve the ffmpeg code and API itself, instead of adding a wrapper for an API that behaves completely different.
Besides, there are some more things missing from the patch. What's the missing? I'm glad to hear it and to fix it.
At least export of metadata like profile, color matrix, and so on (there are a bunch of these, parsed from the h264 bitstream). There's also the question what happens if the hardware can't decode the given video.
The libstagefright decoding wrapper is apparently a piece of shit. Who even uses it? I think libstagefright wrapper is good by giving more options to application, you dislike it, but other may do. The bad of it is on buffer sharing; we solve it by dma_buf.
Shouldn't this use vaapi surfaces (VASurfaceID)? I'm pretty sure this dma_buf stuff is not supported by all backends.
I don't see any nice things. The example player demonstrates how the AVFrame interface will be improved with dma_buf. I do think dma_buf is a good buffer sharing mechanism to promote hw codec. https://github.com/01org/player-ffmpeg-yami
There's already vaapi support in ffmpeg though. While I'm not opposed to improving the hw decoding situation in general. I think it'd be better to improve the ffmpeg code and API itself, instead of adding a wrapper for an API that behaves completely different. I don't know current status of vaapi support in ffmpeg, the following codec are supported by libyami but may not in ffmpeg-vaapi yet: h264enc/vp8dec/vp8enc/vp9dec/jpegdec/jpegenc
to be honest, libyami is core codec library for vaapi (especially on Intel platform). The purpose is to not implement each codec for each media frameworks. Libyami is used in chromeos, the same interface works on android as well; as talked in 0.2.0 release, it can support gstreamer with least effort basing on dma_buf.
At least export of metadata like profile, color matrix, and so on (there are a bunch of these, parsed from the h264 bitstream).
There's also the question what happens if the hardware can't decode the given video. Thanks, we will follow it next.
Shouldn't this use vaapi surfaces (VASurfaceID)? I'm pretty sure this dma_buf stuff is not supported by all backends. VASurfaceID is platform dependent opaque handle, depends on vaapi context to be used; decoder and render are tightly coupled.
while dma_buf is Linux kernel buffer sharing mechanism, it can be better inter-operated among camera/video/egl,opengl/opencl etc. in theory, the only dependency is Linux kernel. dma_buf is not limited to Intel platform as well. Vaapi API and Intel i965 driver starts supporting dma_buf half a year ago, I believe it is the right direction to go. By the way, 4 rendering-modes are supported in my example player; dma_buf is one of them.
-----Original Message----- From: ffmpeg-devel-bounces@ffmpeg.org [mailto:ffmpeg-devel- bounces@ffmpeg.org] On Behalf Of wm4 Sent: Monday, January 12, 2015 9:35 AM To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH 2/2] add libyami.cpp for h264 decoding by libyami
On Mon, 12 Jan 2015 00:46:26 +0000 "Zhao, Halley" <halley.zhao@intel.com> wrote:
Maintaining decoders is the point of this project. Yes, I agree the core of ffmpeg is codec; But, from the user, ffmpeg is usually treated as a light-weight media framework. Being a media framework, it is good to leverage hw codec in many cases.
There's already vaapi support in ffmpeg though. While I'm not opposed to improving the hw decoding situation in general. I think it'd be better to improve the ffmpeg code and API itself, instead of adding a wrapper for an API that behaves completely different.
Besides, there are some more things missing from the patch. What's the missing? I'm glad to hear it and to fix it.
At least export of metadata like profile, color matrix, and so on (there are a bunch of these, parsed from the h264 bitstream).
There's also the question what happens if the hardware can't decode the given video.
The libstagefright decoding wrapper is apparently a piece of shit. Who even uses it? I think libstagefright wrapper is good by giving more options to application, you dislike it, but other may do. The bad of it is on buffer sharing; we solve it by dma_buf.
Shouldn't this use vaapi surfaces (VASurfaceID)? I'm pretty sure this dma_buf stuff is not supported by all backends.
I don't see any nice things. The example player demonstrates how the AVFrame interface will be improved with dma_buf. I do think dma_buf is a good buffer sharing mechanism to promote hw codec. https://github.com/01org/player-ffmpeg-yami
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 12 Jan, Zhao, Halley wrote :
to be honest, libyami is core codec library for vaapi (especially on Intel platform).
FFmpeg already accesses vaapi directly. Adding a library in the middle is just making everything slower... With my kindest regards, -- Jean-Baptiste Kempf http://www.jbkempf.com/ - +33 672 704 734 Sent from my Electronic Device
On Mon, 12 Jan 2015 00:46:26 +0000 "Zhao, Halley" <halley.zhao@intel.com> wrote:
Maintaining decoders is the point of this project. Yes, I agree the core of ffmpeg is codec; But, from the user, ffmpeg is usually treated as a light-weight media framework. Being a media framework, it is good to leverage hw codec in many cases.
i just want to make clear that wm4 is but one of the many devels in ffmpeg and please dont let any one dev scare you off from contributing. -compn
On 12 Jan, Zhao, Halley wrote :
Maintaining decoders is the point of this project. Yes, I agree the core of ffmpeg is codec; But, from the user, ffmpeg is usually treated as a light-weight media framework. Being a media framework, it is good to leverage hw codec in many cases.
FFmpeg already leverage hw codecs.
The libstagefright decoding wrapper is apparently a piece of shit. Who even uses it? I think libstagefright wrapper is good by giving more options to application, you dislike it, but other may do.
libstagefright is a piece of crap, unmaintained by upstream and the integration in FFmpeg is buggy, stupid and unmaintained. Btw, maybe your employer should stop adding media libraries (VAAPI, QSV and yami now) and actually fix them, like the 11-ref frames issues. With my kindest regards, -- Jean-Baptiste Kempf http://www.jbkempf.com/ - +33 672 704 734 Sent from my Electronic Device
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@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -----Original Message----- From: ffmpeg-devel-bounces@ffmpeg.org [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of wm4 Sent: Friday, January 09, 2015 9:45 PM To: ffmpeg-devel@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@intel.com> wrote:
From: "Zhao, Halley" <halley.zhao@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@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@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Wed, 14 Jan 2015 06:53:18 +0000 "Zhao, Halley" <halley.zhao@intel.com> wrote:
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.
So what happens if the profile changes mid-stream? Also, it should return a specific error code. A user might also want to disable decoding for certain profiles, if they're known to be buggy on specific hardware.
+ 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?
Well, in general, sps/pps can be updated mid-stream, so you need to handle it anyway.
+ 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?
As long as the AVCodecContext can be destroyed... Keeping a reference to the decoder seems overly expensive, though. Also, can it happen that the GPU runs out of resources when creating a new decoder while the old one is still not destroyed?
+ 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
But you also wrote:
- do not support multi-thread decoding, it is unnecessary for hw
+ 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?
You're using ff_get_buffer() above, which allocates frame data, and sets frame->buf[0] to the AVBuffer holding this data. If you just overwrite the buffer pointer, the data will never be freed, and you have a frame-sized memory leak on every frame.
On 09.01.2015, at 09:15, "Zhao, Halley" <halley.zhao@intel.com> wrote:
From: "Zhao, Halley" <halley.zhao@intel.com>
- do not support multi-thread decoding, it is unnecessary for hw
While it might not be possible to support in a proper way, and the FFmpeg multithreading is probably useless in the context of this library, but I think this is not generally true. Most hardware decoders need a good bit of parsing, and for some in combination with high-bitrate content it is enough that multithreading can be useful due to allowing to use two CPUs at low frequencies instead of having to clock one significantly higher.
participants (7)
-
compn -
Jean-Baptiste Kempf -
Michael Niedermayer -
Reimar Döffinger -
Ronald S. Bultje -
wm4 -
Zhao, Halley