From 83d687fe70b646e6ab04abf562a94f979beb7af5 Mon Sep 17 00:00:00 2001 From: SeokYeon Hwang Date: Wed, 10 Sep 2014 15:32:13 +0900 Subject: [PATCH] brillcodec: source readability improvement Apply data handler to video encoder to reduce memory copy. Use struct instead of access via memory offset. Change-Id: I9ae8cc808ce36e0223fe7dd93c972043431e70cc Signed-off-by: SeokYeon Hwang --- tizen/src/hw/pci/maru_brillcodec.c | 437 +++++++++++++----------------- tizen/src/hw/pci/maru_brillcodec.h | 5 + tizen/src/hw/pci/maru_brillcodec_plugin.h | 41 --- 3 files changed, 199 insertions(+), 284 deletions(-) diff --git a/tizen/src/hw/pci/maru_brillcodec.c b/tizen/src/hw/pci/maru_brillcodec.c index aa3daa1..efd1dd4 100644 --- a/tizen/src/hw/pci/maru_brillcodec.c +++ b/tizen/src/hw/pci/maru_brillcodec.c @@ -84,6 +84,75 @@ struct audio_data { int64_t channel_layout; }; +struct video_data { + int32_t width; + int32_t height; + int32_t fps_n; + int32_t fps_d; + int32_t par_n; + int32_t par_d; + int32_t pix_fmt; + int32_t bpp; + int32_t ticks_per_frame; +} __attribute__((packed)); + +struct video_decode_input { + int32_t inbuf_size; + int32_t idx; + int64_t in_offset; + uint8_t inbuf; // for pointing inbuf address +} __attribute__((packed)); + +struct video_decode_output { + int32_t len; + int32_t got_picture; + struct video_data data; +} __attribute__((packed)); + +struct video_encode_input { + int32_t inbuf_size; + int64_t in_timestamp; + uint8_t inbuf; // for pointing inbuf address +} __attribute__((packed)); + +struct video_encode_output { + int32_t len; + int32_t coded_frame; + int32_t key_frame; + uint8_t data; // for pointing outbuf address +} __attribute__((packed)); + +typedef struct DataContainer { + // common + bool is_got; + int32_t len; + AVCodecContext *avctx; + + // for decoder + size_t picture_buffer_offset; + AVFrame *frame; + + // for encoder + AVPacket *avpkt; +} DataContainer; + +static void fill_video_data(const AVCodecContext *avctx, + struct video_data *video) +{ + memset(video, 0x00, sizeof(struct video_data)); + + video->width = avctx->width; + video->height = avctx->height; + video->fps_n = avctx->time_base.num; + video->fps_d = avctx->time_base.den; + video->pix_fmt = avctx->pix_fmt; + video->par_n = avctx->sample_aspect_ratio.num; + video->par_d = avctx->sample_aspect_ratio.den; + video->bpp = avctx->bits_per_coded_sample; + video->ticks_per_frame = avctx->ticks_per_frame; +} + + DeviceMemEntry *entry[CODEC_CONTEXT_MAX]; // define a queue to manage ioparam, context data @@ -161,31 +230,28 @@ static void default_get_picture(void *dst, void *src, enum AVPixelFormat pix_fmt frame->width, frame->height, dst, pict_size); } -// default video decode data handler +// video decode data handler // FIXME: ignore "size" now... -static void copy_picture(void *dst, void *opaque, size_t dummy) +static void copy_decode_data(void *dst, void *opaque, size_t dummy) { - size_t size = sizeof(int32_t), offset = 0; DataContainer *dc = (DataContainer *)opaque; CodecContext *context = (CodecContext *)dc->avctx->opaque; if (dc->picture_buffer_offset) { - // FIXME: if video data is exist... - *((int32_t *)dst) = dc->len; - offset += size; - *((int32_t *)(dst + offset)) = dc->got_picture; - offset += size; - - struct video_data *data = (struct video_data *)(dst + offset); - fill_video_data(dc->avctx, data); + // if output video data is exist... + struct video_decode_output *decode_output = + (struct video_decode_output *)dst; + decode_output->len = dc->len; + decode_output->got_picture = dc->is_got ? 1 : 0; + fill_video_data(dc->avctx, &decode_output->data); if (context->is_hwaccel) { - data->pix_fmt = context->state->hwaccel_plugin->output_pix_fmt; + decode_output->data.pix_fmt = context->state->hwaccel_plugin->output_pix_fmt; } } if (dc->frame) { - // FIXME: if picture is exist... + // if picture is exist... if (context->is_hwaccel) { context->state->hwaccel_plugin->get_picture(dst + dc->picture_buffer_offset, dc->frame); } else { @@ -196,11 +262,40 @@ static void copy_picture(void *dst, void *opaque, size_t dummy) static void release(void *opaque) { DataContainer *dc = (DataContainer *)opaque; + if (dc->avpkt) { + g_free(dc->avpkt->data); + g_free(dc->avpkt); + } g_free(dc); } static DataHandler video_decode_data_handler = { - .get_data = copy_picture, + .get_data = copy_decode_data, + .release = release, +}; + +static void copy_encode_data(void *dst, void *opaque, size_t dummy) +{ + DataContainer *dc = (DataContainer *)opaque; + struct video_encode_output *encode_output = + (struct video_encode_output *)dst; + + encode_output->len = dc->len; + if (dc->len && dc->is_got) { + // inform gstreamer plugin about the status of encoded frames + // A flag for output buffer in gstreamer is depending on the status. + if (dc->avctx->coded_frame) { + encode_output->coded_frame = 1; + // if key_frame is 0, this frame cannot be decoded independently. + encode_output->key_frame = dc->avctx->coded_frame->key_frame; + } + + memcpy(&encode_output->data, dc->avpkt->data, dc->avpkt->size); + } +} + +static DataHandler video_encode_data_handler = { + .get_data = copy_encode_data, .release = release, }; @@ -234,10 +329,6 @@ void brillcodec_wakeup_threads(MaruBrillCodecState *s, int api_index) CodecParam *ioparam = NULL; ioparam = g_malloc0(sizeof(CodecParam)); - if (!ioparam) { - ERR("failed to allocate ioparam\n"); - return; - } memcpy(ioparam, &s->ioparam, sizeof(CodecParam)); @@ -373,10 +464,6 @@ static void brillcodec_push_readqueue(MaruBrillCodecState *s, DeviceMemEntry *data_buf = NULL; elem = g_malloc0(sizeof(CodecDataStg)); - if (!elem) { - ERR("failed to allocate ioparam_queue. %d\n", sizeof(CodecDataStg)); - return; - } elem->param_buf = ioparam; @@ -406,11 +493,6 @@ static void *brillcodec_store_inbuf(uint8_t *mem_base, uint8_t *device_mem = mem_base + ioparam->mem_offset; elem = g_malloc0(sizeof(DeviceMemEntry)); - if (!elem) { - ERR("failed to allocate readqueue node. size: %d\n", - sizeof(DeviceMemEntry)); - return NULL; - } memcpy(&readbuf_size, device_mem, sizeof(readbuf_size)); size = sizeof(readbuf_size); @@ -421,13 +503,9 @@ static void *brillcodec_store_inbuf(uint8_t *mem_base, ioparam->api_index, ioparam->ctx_index, ioparam->mem_offset); } else { readbuf = g_malloc0(readbuf_size); - if (!readbuf) { - ERR("failed to allocate a read buffer. size: %d\n", readbuf_size); - } else { - TRACE("copy input buffer from guest. ctx_id: %d, mem_offset: %x\n", + TRACE("copy input buffer from guest. ctx_id: %d, mem_offset: %x\n", ioparam->ctx_index, ioparam->mem_offset); - memcpy(readbuf, device_mem + size, readbuf_size); - } + memcpy(readbuf, device_mem + size, readbuf_size); } // memset(device_mem, 0x00, sizeof(readbuf_size)); @@ -1238,69 +1316,56 @@ static bool codec_flush_buffers(MaruBrillCodecState *s, int ctx_id, void *data_b return ret; } -static bool codec_decode_video2(MaruBrillCodecState *s, int ctx_id, void *data_buf) +static bool codec_decode_video_common(MaruBrillCodecState *s, int ctx_id, + void *data_buf, bool copy_picture) { AVCodecContext *avctx = NULL; - AVFrame *picture = NULL; + AVFrame *frame = NULL; AVCodecParserContext *pctx = NULL; AVPacket avpkt; - uint32_t got_picture = 0, len = -1; - uint8_t *inbuf = NULL; - int inbuf_size = 0, idx = 0, size = 0; - int64_t in_offset = 0; DeviceMemEntry *elem = NULL; + struct video_decode_input empty_input = { 0, }; + struct video_decode_input *decode_input = &empty_input; + uint32_t got_picture = 0; + int32_t len = -1; TRACE("enter: %s\n", __func__); elem = (DeviceMemEntry *)data_buf; - if (elem && elem->opaque) { - memcpy(&inbuf_size, elem->opaque, sizeof(inbuf_size)); - size += sizeof(inbuf_size); - memcpy(&idx, elem->opaque + size, sizeof(idx)); - size += sizeof(idx); - memcpy(&in_offset, elem->opaque + size, sizeof(in_offset)); - size += sizeof(in_offset); - TRACE("decode_video. inbuf_size %d\n", inbuf_size); - - if (inbuf_size > 0) { - inbuf = elem->opaque + size; - } - } else { + if (!elem || !elem->opaque) { TRACE("decode_video. no input buffer\n"); - // FIXME: improve error handling - // return false; + } + else { + decode_input = elem->opaque; } av_init_packet(&avpkt); - avpkt.data = inbuf; - avpkt.size = inbuf_size; + avpkt.data = &decode_input->inbuf; + avpkt.size = decode_input->inbuf_size; avctx = CONTEXT(s, ctx_id)->avctx; - picture = CONTEXT(s, ctx_id)->frame; - if (!avctx) { - ERR("decode_video. %d of AVCodecContext is NULL.\n", ctx_id); - } else if (!avctx->codec) { - ERR("decode_video. %d of AVCodec is NULL.\n", ctx_id); - } else if (!picture) { - ERR("decode_video. %d of AVFrame is NULL.\n", ctx_id); - } else { - TRACE("decode_video. bitrate %d resolution(%dx%d)\n", - avctx->bit_rate, avctx->width, avctx->height); - - pctx = CONTEXT(s, ctx_id)->parser_ctx; + frame = CONTEXT(s, ctx_id)->frame; + pctx = CONTEXT(s, ctx_id)->parser_ctx; - len = parse_and_decode_video(avctx, picture, pctx, ctx_id, - &avpkt, &got_picture, idx, in_offset); + if(!avctx || !avctx->codec || !frame) { + ERR("critical error !!!\n"); + assert(0); } + TRACE("decode_video. bitrate %d resolution(%dx%d)\n", + avctx->bit_rate, avctx->width, avctx->height); + + len = parse_and_decode_video(avctx, frame, pctx, ctx_id, + &avpkt, &got_picture, decode_input->idx, decode_input->in_offset); + DataContainer *dc = g_malloc0(sizeof(DataContainer)); - dc->picture_buffer_offset = OFFSET_PICTURE_BUFFER; + dc->picture_buffer_offset = OFFSET_PICTURE_BUFFER; // we have output video data dc->len = len; - dc->got_picture = got_picture; + dc->is_got = got_picture; dc->avctx = avctx; - if(got_picture) { - dc->frame = picture; + if(got_picture && copy_picture) { // we have output picture + dc->frame = frame; } brillcodec_push_write_queue(s, dc, 0, ctx_id, &video_decode_data_handler); @@ -1312,97 +1377,30 @@ static bool codec_decode_video2(MaruBrillCodecState *s, int ctx_id, void *data_b static bool codec_decode_video(MaruBrillCodecState *s, int ctx_id, void *data_buf) { - AVCodecContext *avctx = NULL; - AVFrame *picture = NULL; - AVCodecParserContext *pctx = NULL; - AVPacket avpkt; - - uint32_t got_picture = 0, len = -1; - uint8_t *inbuf = NULL; - int inbuf_size = 0, idx = 0, size = 0; - int64_t in_offset = 0; - DeviceMemEntry *elem = NULL; - - TRACE("enter: %s\n", __func__); - - elem = (DeviceMemEntry *)data_buf; - if (elem && elem->opaque) { - memcpy(&inbuf_size, elem->opaque, sizeof(inbuf_size)); - size += sizeof(inbuf_size); - memcpy(&idx, elem->opaque + size, sizeof(idx)); - size += sizeof(idx); - memcpy(&in_offset, elem->opaque + size, sizeof(in_offset)); - size += sizeof(in_offset); - TRACE("decode_video. inbuf_size %d\n", inbuf_size); - - if (inbuf_size > 0) { - inbuf = elem->opaque + size; - } - } else { - TRACE("decode_video. no input buffer\n"); - // FIXME: improve error handling - // return false; - } - - av_init_packet(&avpkt); - avpkt.data = inbuf; - avpkt.size = inbuf_size; - - avctx = CONTEXT(s, ctx_id)->avctx; - picture = CONTEXT(s, ctx_id)->frame; - if (!avctx) { - ERR("decode_video. %d of AVCodecContext is NULL.\n", ctx_id); - } else if (!avctx->codec) { - ERR("decode_video. %d of AVCodec is NULL.\n", ctx_id); - } else if (!picture) { - ERR("decode_video. %d of AVFrame is NULL.\n", ctx_id); - } else { - TRACE("decode_video. bitrate %d resolution(%dx%d)\n", - avctx->bit_rate, avctx->width, avctx->height); - - pctx = CONTEXT(s, ctx_id)->parser_ctx; - - len = parse_and_decode_video(avctx, picture, pctx, ctx_id, - &avpkt, &got_picture, idx, in_offset); - } - - DataContainer *dc = g_malloc0(sizeof(DataContainer)); - dc->picture_buffer_offset = OFFSET_PICTURE_BUFFER; - dc->len = len; - dc->got_picture = got_picture; - dc->avctx = avctx; - - brillcodec_push_write_queue(s, dc, 0, ctx_id, &video_decode_data_handler); - - TRACE("leave: %s\n", __func__); + return codec_decode_video_common(s, ctx_id, data_buf, false); +} - return true; +static bool codec_decode_video2(MaruBrillCodecState *s, int ctx_id, void *data_buf) +{ + return codec_decode_video_common(s, ctx_id, data_buf, true); } -// for old decode API -static bool codec_picture_copy (MaruBrillCodecState *s, int ctx_id, void *elem) +static bool codec_picture_copy(MaruBrillCodecState *s, int ctx_id, void *elem) { - AVCodecContext *avctx = NULL; - AVFrame *frame = NULL; - bool ret = true; + DataContainer *dc = g_malloc0(sizeof(DataContainer)); TRACE("enter: %s\n", __func__); TRACE("copy decoded image of %d context.\n", ctx_id); - avctx = CONTEXT(s, ctx_id)->avctx; - frame = CONTEXT(s, ctx_id)->frame; - - DataContainer *dc = g_malloc0(sizeof(DataContainer)); - - dc->frame = frame; - dc->avctx = avctx; + dc->avctx = CONTEXT(s, ctx_id)->avctx; + dc->frame = CONTEXT(s, ctx_id)->frame; brillcodec_push_write_queue(s, dc, 0, ctx_id, &video_decode_data_handler); TRACE("leave: %s\n", __func__); - return ret; + return true; } static bool codec_decode_audio(MaruBrillCodecState *s, int ctx_id, void *data_buf) @@ -1521,7 +1519,6 @@ static bool codec_decode_audio(MaruBrillCodecState *s, int ctx_id, void *data_bu av_free(out_buf); } - TRACE("leave: %s\n", __func__); return true; } @@ -1530,124 +1527,78 @@ static bool codec_encode_video(MaruBrillCodecState *s, int ctx_id, void *data_bu { AVCodecContext *avctx = NULL; AVFrame *pict = NULL; - AVPacket avpkt; + AVPacket *avpkt = g_malloc0(sizeof(AVPacket)); uint8_t *inbuf = NULL, *outbuf = NULL; - int inbuf_size = 0, outbuf_size = 0; - int got_frame = 0, ret = 0, size = 0; - int64_t in_timestamp = 0; - int coded_frame = 0, key_frame = 0; + int outbuf_size = 0; + int got_frame = 0, ret = 0; DeviceMemEntry *elem = NULL; - uint8_t *tempbuf = NULL; - int tempbuf_size = 0; + struct video_encode_input empty_input = { 0, }; + struct video_encode_input *encode_input = &empty_input; TRACE("enter: %s\n", __func__); elem = (DeviceMemEntry *)data_buf; - if (elem && elem->opaque) { - memcpy(&inbuf_size, elem->opaque, sizeof(inbuf_size)); - size += sizeof(inbuf_size); - memcpy(&in_timestamp, elem->opaque + size, sizeof(in_timestamp)); - size += sizeof(in_timestamp); - TRACE("encode video. inbuf_size %d\n", inbuf_size); - - if (inbuf_size > 0) { - inbuf = elem->opaque + size; - } - } else { - TRACE("encode video. no input buffer.\n"); - // FIXME: improve error handling - // return false; + if (!elem || !elem->opaque) { + TRACE("encode_video. no input buffer\n"); + } + else { + encode_input = elem->opaque; } // initialize AVPacket - av_init_packet(&avpkt); - avpkt.data = NULL; - avpkt.size = 0; + av_init_packet(avpkt); avctx = CONTEXT(s, ctx_id)->avctx; pict = CONTEXT(s, ctx_id)->frame; - if (!avctx || !pict) { - ERR("%d of context or frame is NULL\n", ctx_id); - } else if (!avctx->codec) { - ERR("%d of AVCodec is NULL.\n", ctx_id); - } else { - TRACE("pixel format: %d inbuf: %p, picture data: %p\n", + + if(!avctx || !avctx->codec) { + ERR("critical error !!!\n"); + assert(0); + } + + TRACE("pixel format: %d inbuf: %p, picture data: %p\n", avctx->pix_fmt, inbuf, pict->data[0]); - ret = avpicture_fill((AVPicture *)pict, inbuf, avctx->pix_fmt, - avctx->width, avctx->height); - if (ret < 0) { - ERR("after avpicture_fill, ret:%d\n", ret); + ret = avpicture_fill((AVPicture *)pict, &encode_input->inbuf, avctx->pix_fmt, + avctx->width, avctx->height); + if (ret < 0) { + ERR("after avpicture_fill, ret:%d\n", ret); + } else { + if (avctx->time_base.num == 0) { + pict->pts = AV_NOPTS_VALUE; } else { - if (avctx->time_base.num == 0) { - pict->pts = AV_NOPTS_VALUE; - } else { - AVRational bq = - {1, (G_USEC_PER_SEC * G_GINT64_CONSTANT(1000))}; - pict->pts = av_rescale_q(in_timestamp, bq, avctx->time_base); - } - TRACE("encode video. ticks_per_frame:%d, pts:%lld\n", + AVRational bq = + {1, (G_USEC_PER_SEC * G_GINT64_CONSTANT(1000))}; + pict->pts = av_rescale_q(encode_input->in_timestamp, bq, avctx->time_base); + } + TRACE("encode video. ticks_per_frame:%d, pts:%lld\n", avctx->ticks_per_frame, pict->pts); - outbuf_size = - (avctx->width * avctx->height * 6) + FF_MIN_BUFFER_SIZE; + outbuf_size = + (avctx->width * avctx->height * 6) + FF_MIN_BUFFER_SIZE; - outbuf = g_malloc0(outbuf_size); + outbuf = g_malloc0(outbuf_size); - avpkt.data = outbuf; - avpkt.size = outbuf_size; + avpkt->data = outbuf; + avpkt->size = outbuf_size; - if (!outbuf) { - ERR("failed to allocate a buffer of encoding video.\n"); - } else { - ret = avcodec_encode_video2(avctx, &avpkt, pict, &got_frame); + ret = avcodec_encode_video2(avctx, avpkt, pict, &got_frame); - TRACE("encode video. ret %d got_picture %d outbuf_size %d\n", ret, got_frame, avpkt.size); - if (avctx->coded_frame) { - TRACE("encode video. keyframe %d\n", avctx->coded_frame->key_frame); - } - } + TRACE("encode video. ret %d got_frame %d outbuf_size %d\n", ret, got_frame, avpkt->size); + if (avctx->coded_frame) { + TRACE("encode video. keyframe %d\n", avctx->coded_frame->key_frame); } } - tempbuf_size = sizeof(ret); - if (ret < 0) { - ERR("failed to encode video. ctx_id %d ret %d\n", ctx_id, ret); - } else { - tempbuf_size += avpkt.size + sizeof(coded_frame) + sizeof(key_frame); - } - // write encoded video data - tempbuf = g_malloc0(tempbuf_size); - if (!tempbuf) { - ERR("encode video. failed to allocate encoded out buffer.\n"); - } else { - memcpy(tempbuf, &avpkt.size, sizeof(avpkt.size)); - size = sizeof(avpkt.size); - - if ((got_frame) && outbuf) { - // inform gstreamer plugin about the status of encoded frames - // A flag for output buffer in gstreamer is depending on the status. - if (avctx->coded_frame) { - coded_frame = 1; - // if key_frame is 0, this frame cannot be decoded independently. - key_frame = avctx->coded_frame->key_frame; - } - memcpy(tempbuf + size, &coded_frame, sizeof(coded_frame)); - size += sizeof(coded_frame); - memcpy(tempbuf + size, &key_frame, sizeof(key_frame)); - size += sizeof(key_frame); - memcpy(tempbuf + size, outbuf, avpkt.size); - } - } - - if (outbuf) { - TRACE("release encoded output buffer. %p\n", outbuf); - g_free(outbuf); - } + DataContainer *dc = g_malloc0(sizeof(DataContainer)); + dc->len = ret; + dc->is_got = got_frame; + dc->avctx = avctx; + dc->avpkt = avpkt; - brillcodec_push_write_queue(s, tempbuf, tempbuf_size, ctx_id, NULL); + brillcodec_push_write_queue(s, dc, 0, ctx_id, &video_encode_data_handler); TRACE("leave: %s\n", __func__); return true; diff --git a/tizen/src/hw/pci/maru_brillcodec.h b/tizen/src/hw/pci/maru_brillcodec.h index 026dca3..161fcbe 100644 --- a/tizen/src/hw/pci/maru_brillcodec.h +++ b/tizen/src/hw/pci/maru_brillcodec.h @@ -101,6 +101,11 @@ struct MaruBrillCodecState { CodecPlugin *hwaccel_plugin; }; +typedef struct DataHandler { + void (*get_data)(void *dst, void *src, size_t size); + void (*release)(void *opaque); +} DataHandler; + typedef struct DeviceMemEntry { void *opaque; uint32_t data_size; diff --git a/tizen/src/hw/pci/maru_brillcodec_plugin.h b/tizen/src/hw/pci/maru_brillcodec_plugin.h index 889a328..5b49b24 100644 --- a/tizen/src/hw/pci/maru_brillcodec_plugin.h +++ b/tizen/src/hw/pci/maru_brillcodec_plugin.h @@ -35,19 +35,6 @@ #define OFFSET_PICTURE_BUFFER (0x100) -typedef struct DataContainer { - size_t picture_buffer_offset; - uint32_t len; - uint32_t got_picture; - AVCodecContext *avctx; - AVFrame *frame; -} DataContainer; - -typedef struct DataHandler { - void (*get_data)(void *dst, void *src, size_t size); - void (*release)(void *opaque); -} DataHandler; - typedef struct CodecPlugin { enum PixelFormat pix_fmt; enum PixelFormat output_pix_fmt; @@ -57,32 +44,4 @@ typedef struct CodecPlugin { void (*get_picture)(void *dst, void *src); } CodecPlugin; -struct video_data { - int32_t width; - int32_t height; - int32_t fps_n; - int32_t fps_d; - int32_t par_n; - int32_t par_d; - int32_t pix_fmt; - int32_t bpp; - int32_t ticks_per_frame; -}; - -static inline void fill_video_data(const AVCodecContext *avctx, - struct video_data *video) -{ - memset(video, 0x00, sizeof(struct video_data)); - - video->width = avctx->width; - video->height = avctx->height; - video->fps_n = avctx->time_base.num; - video->fps_d = avctx->time_base.den; - video->pix_fmt = avctx->pix_fmt; - video->par_n = avctx->sample_aspect_ratio.num; - video->par_d = avctx->sample_aspect_ratio.den; - video->bpp = avctx->bits_per_coded_sample; - video->ticks_per_frame = avctx->ticks_per_frame; -} - #endif //__MARU_BRILLCODEC_PLUGIN_H__ -- 2.7.4