On Sat, 16 Dec 2017 20:26:56 +0800 Wang Bin <wbse...@gmail.com> wrote:
> 2017-12-16 19:47 GMT+08:00 wm4 <nfx...@googlemail.com>: > > On Sat, 16 Dec 2017 13:48:05 +0800 > > Wang Bin <wbse...@gmail.com> wrote: > > > >> 2017-12-16 2:50 GMT+08:00 wm4 <nfx...@googlemail.com>: > >> > On Fri, 15 Dec 2017 15:05:50 +0800 > >> > wbse...@gmail.com wrote: > >> > > >> >> From: wang-bin <wbse...@gmail.com> > >> >> > >> >> mmal buffer->data is already in host memory. AFAIK decoders implemented > >> >> in omx must > >> >> be configured to output frames to either memory or something directly > >> >> used by renderer, > >> >> for example mediacodec surface, mmal buffer and omxil eglimage. > >> >> test result: big buck bunny 1080p fps increases from about 100 to 110 > >> >> if copy_frame is > >> >> turned off > >> >> --- > >> >> libavcodec/mmaldec.c | 31 +++++++++++++++++++++++-------- > >> >> 1 file changed, 23 insertions(+), 8 deletions(-) > >> >> > >> >> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c > >> >> index c1cfb09283..9cd6c6558f 100644 > >> >> --- a/libavcodec/mmaldec.c > >> >> +++ b/libavcodec/mmaldec.c > >> >> @@ -69,6 +69,7 @@ typedef struct MMALDecodeContext { > >> >> AVClass *av_class; > >> >> int extra_buffers; > >> >> int extra_decoder_buffers; > >> >> + int copy_frame; > >> >> > >> >> MMAL_COMPONENT_T *decoder; > >> >> MMAL_QUEUE_T *queue_decoded_frames; > >> >> @@ -139,7 +140,6 @@ static int ffmmal_set_ref(AVFrame *frame, FFPoolRef > >> >> *pool, > >> >> atomic_fetch_add_explicit(&ref->pool->refcount, 1, > >> >> memory_order_relaxed); > >> >> mmal_buffer_header_acquire(buffer); > >> >> > >> >> - frame->format = AV_PIX_FMT_MMAL; > >> >> frame->data[3] = (uint8_t *)ref->buffer; > >> >> return 0; > >> >> } > >> >> @@ -650,20 +650,34 @@ static int ffmal_copy_frame(AVCodecContext > >> >> *avctx, AVFrame *frame, > >> >> > >> >> if ((ret = ffmmal_set_ref(frame, ctx->pool_out, buffer)) < 0) > >> >> goto done; > >> >> + frame->format = AV_PIX_FMT_MMAL; > >> >> } else { > >> >> int w = FFALIGN(avctx->width, 32); > >> >> int h = FFALIGN(avctx->height, 16); > >> >> uint8_t *src[4]; > >> >> int linesize[4]; > >> >> > >> >> - if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) > >> >> - goto done; > >> >> + if (ctx->copy_frame) { > >> >> + if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) > >> >> + goto done; > >> >> > >> >> - av_image_fill_arrays(src, linesize, > >> >> - buffer->data + > >> >> buffer->type->video.offset[0], > >> >> - avctx->pix_fmt, w, h, 1); > >> >> - av_image_copy(frame->data, frame->linesize, src, linesize, > >> >> - avctx->pix_fmt, avctx->width, avctx->height); > >> >> + av_image_fill_arrays(src, linesize, > >> >> + buffer->data + > >> >> buffer->type->video.offset[0], > >> >> + avctx->pix_fmt, w, h, 1); > >> >> + av_image_copy(frame->data, frame->linesize, src, linesize, > >> >> + avctx->pix_fmt, avctx->width, avctx->height); > >> >> + } else { > >> >> + if ((ret = ff_decode_frame_props(avctx, frame)) < 0) > >> >> + goto done; > >> >> + /* buffer->type->video.offset/pitch[i]; is always 0 */ > >> >> + av_image_fill_arrays(src, linesize, > >> >> + buffer->data + > >> >> buffer->type->video.offset[0], > >> >> + avctx->pix_fmt, w, h, 1); > >> >> + if ((ret = ffmmal_set_ref(frame, ctx->pool_out, buffer)) < > >> >> 0) > >> >> + goto done; > >> >> + memcpy(frame->data, src, sizeof(src)); > >> >> + memcpy(frame->linesize, linesize, sizeof(linesize)); > >> >> + } > >> >> } > >> >> > >> >> frame->pts = buffer->pts == MMAL_TIME_UNKNOWN ? AV_NOPTS_VALUE : > >> >> buffer->pts; > >> >> @@ -842,6 +856,7 @@ AVHWAccel ff_wmv3_mmal_hwaccel = { > >> >> static const AVOption options[]={ > >> >> {"extra_buffers", "extra buffers", offsetof(MMALDecodeContext, > >> >> extra_buffers), AV_OPT_TYPE_INT, {.i64 = 10}, 0, 256, 0}, > >> >> {"extra_decoder_buffers", "extra MMAL internal buffered frames", > >> >> offsetof(MMALDecodeContext, extra_decoder_buffers), AV_OPT_TYPE_INT, > >> >> {.i64 = 10}, 0, 256, 0}, > >> >> + {"copy_frame", "copy deocded data to avframe", > >> >> offsetof(MMALDecodeContext, copy_frame), AV_OPT_TYPE_BOOL, {.i64 = 1}, > >> >> 0, 256, 0}, > >> >> {NULL} > >> >> }; > >> >> > >> > > >> > Didn't check too closely what exactly the patch does, but adding an > >> > option for it sounds very wrong. The user select in the get_format > >> > callback whether a GPU surface is output (MMAL pixfmt), or software. > >> > >> Avoid copying data from mmal buffer->data to avframe data. Instead, > >> just fill strides and address of each plane in avframe, and add a > >> reference to mmal buffer. > > > > Does it make sure to keep the mmal buffer pool alive then? Otherwise a > > decoded AVFrame would become invalid after closing and destroying the > > decoder. > > Yes. ffmmal_set_ref() is called like hw pixfmt code and mmal buffer's > reference is increased. otherwise the displayed frame is weired > because the buffer may be invalid as you say. I tested it in mpv. > > > Why would this need a new option? > > For testing purposes. You can compare the performance with the option. Fine then I guess. It could still be surprising to the API user that holding a frame will keep a large chunk of memory alive (the whole pool), but I guess that's unavoidable with mmal's design. The API user could still copy the frame manually if it becomes a problem. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel