[FFmpeg-devel] [PATCH v18 06/10] avcodec/evc_decoder: Provided support for EVC decoder
James Almer
jamrial at gmail.com
Wed Mar 29 15:16:49 EEST 2023
On 3/28/2023 10:50 AM, Dawid Kozinski wrote:
> +static int libxevd_receive_frame(AVCodecContext *avctx, AVFrame *frame)
> +{
> + XevdContext *xectx = NULL;
> + AVPacket *pkt = NULL;
> + XEVD_IMGB *imgb = NULL;
> +
> + int xevd_ret = 0;
> + int ret = 0;
> +
> + xectx = avctx->priv_data;
> +
> + pkt = av_packet_alloc();
> + if (!pkt)
> + return AVERROR(ENOMEM);
This is very inefficient. Just keep an AVPacket in the XevdContext,
allocating it on init(), unrefing it after being used in this function,
and freeing it on close().
> +
> + // obtain input data
> + ret = ff_decode_get_packet(avctx, pkt);
> + if (ret < 0 && ret != AVERROR_EOF) {
> + av_packet_free(&pkt);
> + return ret;
> + } else if(ret == AVERROR_EOF && xectx->draining_mode == 0) { // End of stream situations. Enter draining mode
> +
> + frame = NULL;
> + xectx->draining_mode = 1;
> +
> + av_packet_free(&pkt);
> +
> + return AVERROR_EOF;
By returning EOF here you're effectively ending the decoding process. No
draining is taking place. This function is not going to be called again
for the xectx->draining_mode == 1 to take effect.
> + }
> +
> + if (pkt->size > 0) {
> + int bs_read_pos = 0;
> + XEVD_STAT stat;
> + XEVD_BITB bitb;
> + int nalu_size;
> +
> + imgb = NULL;
> +
> + while(pkt->size > (bs_read_pos + XEVD_NAL_UNIT_LENGTH_BYTE)) {
> + memset(&stat, 0, sizeof(XEVD_STAT));
> +
> + nalu_size = read_nal_unit_length(pkt->data + bs_read_pos, XEVD_NAL_UNIT_LENGTH_BYTE, avctx);
> + if (nalu_size == 0) {
> + av_log(avctx, AV_LOG_ERROR, "Invalid bitstream\n");
> + av_packet_free(&pkt);
> + ret = AVERROR_INVALIDDATA;
> + return ret;
> + }
> + bs_read_pos += XEVD_NAL_UNIT_LENGTH_BYTE;
> +
> + bitb.addr = pkt->data + bs_read_pos;
> + bitb.ssize = nalu_size;
> +
> + /* main decoding block */
> + xevd_ret = xevd_decode(xectx->id, &bitb, &stat);
> + if (XEVD_FAILED(xevd_ret)) {
> + av_log(avctx, AV_LOG_ERROR, "Failed to decode bitstream\n");
> + av_packet_free(&pkt);
> + ret = AVERROR_EXTERNAL;
> + return ret;
> + }
> +
> + bs_read_pos += nalu_size;
> +
> + if (stat.nalu_type == XEVD_NUT_SPS) { // EVC stream parameters changed
> + if ((ret = export_stream_params(xectx, avctx)) != 0) {
> + av_log(avctx, AV_LOG_ERROR, "Failed to export stream params\n");
> + av_packet_free(&pkt);
> + return ret;
> + }
> + }
> + if (stat.read != nalu_size)
> + av_log(avctx, AV_LOG_INFO, "Different reading of bitstream (in:%d,read:%d)\n,", nalu_size, stat.read);
> + if (stat.fnum >= 0) {
> + // already has a decoded image
> + if (imgb) {
> + // xevd_pull uses pool of objects of type XEVD_IMGB.
> + // The pool size is equal MAX_PB_SIZE (26), so release object when it is no more needed
> + imgb->release(imgb);
> + imgb = NULL;
Aren't you dropping an image you should be propagating instead with this?
> + }
> + xevd_ret = xevd_pull(xectx->id, &imgb);
> + if (XEVD_FAILED(xevd_ret)) {
> + av_log(avctx, AV_LOG_ERROR, "Failed to pull the decoded image (xevd error code: %d, frame#=%d)\n", xevd_ret, stat.fnum);
> + ret = AVERROR_EXTERNAL;
> + av_packet_free(&pkt);
> +
> + return ret;
> + } else if (xevd_ret == XEVD_OK_FRM_DELAYED) {
> + if (imgb) {
> + // xevd_pull uses pool of objects of type XEVD_IMGB.
> + // The pool size is equal MAX_PB_SIZE (26), so release object when it is no more needed
> + imgb->release(imgb);
> + imgb = NULL;
Can an image be returned when xevd_ret == XEVD_OK_FRM_DELAYED? If so,
shouldn't you propagate it?
> + }
> + av_packet_free(&pkt);
> +
> + return AVERROR(EAGAIN);
> + }
> + if (imgb) { // got frame
> + int ret = libxevd_image_copy(avctx, imgb, frame);
> + if(ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Image copying error\n");
> + if (imgb) {
> + imgb->release(imgb);
> + imgb = NULL;
> + }
> + av_packet_free(&pkt);
> +
> + return ret;
> + }
> +
> + // use ff_decode_frame_props() to fill frame properties
> + ret = ff_decode_frame_props(avctx, frame);
> + if (ret < 0) {
> + if (imgb) {
> + imgb->release(imgb);
> + imgb = NULL;
> + }
> + av_packet_free(&pkt);
> + av_frame_unref(frame);
> +
> + return ret;
> + }
> + // match timestamps and packet size
> + ret = ff_decode_frame_props_from_pkt(avctx, frame, pkt);
> + pkt->opaque = NULL;
You copy-pasted this from libdav1d.c without knowing what it's doing.
You don't need to clear opaque, and you don't need to call
ff_decode_frame_props_from_pkt() with the current packet when
ff_decode_frame_props() is already doing it for you. The former function
is only needed when you need props from a packet other than the last one
fed to the decoder.
> + if (ret < 0) {
> + if (imgb) {
> + imgb->release(imgb);
> + imgb = NULL;
> + }
> + av_packet_free(&pkt);
> + av_frame_unref(frame);
> +
> + return ret;
> + }
> +
> + frame->pkt_dts = pkt->dts;
> +
> + // xevd_pull uses pool of objects of type XEVD_IMGB.
> + // The pool size is equal MAX_PB_SIZE (26), so release object when it is no more needed
> + imgb->release(imgb);
> + imgb = NULL;
> +
> + return 0;
> + } else
> + return AVERROR(EAGAIN);
> + }
> + }
> + } else { // decoder draining mode handling
> + xevd_ret = xevd_pull(xectx->id, &imgb);
> + if (xevd_ret == XEVD_ERR_UNEXPECTED) // draining process completed
> + return AVERROR_EOF;
> + else if (XEVD_FAILED(xevd_ret)) {
> + av_log(avctx, AV_LOG_ERROR, "Failed to pull the decoded image (xevd error code: %d)\n", xevd_ret);
> +
> + if (imgb) {
> + imgb->release(imgb);
> + imgb = NULL;
> + }
> +
> + av_packet_free(&pkt);
> +
> + return AVERROR_EXTERNAL;
> + }
> + if (imgb) { // got frame
> + int ret = libxevd_image_copy(avctx, imgb, frame);
> + if(ret < 0) {
> + if (imgb) {
> + imgb->release(imgb);
> + imgb = NULL;
> + }
> + av_packet_free(&pkt);
> + }
> +
> + frame->pkt_dts = pkt->dts;
> +
> + // xevd_pull uses pool of objects of type XEVD_IMGB.
> + // The pool size is equal MAX_PB_SIZE (26), so release object when it is no more needed
> + imgb->release(imgb);
> + imgb = NULL;
> +
> + return 0;
> + }
> + return AVERROR_EOF;
> + }
> + return ret;
> +}
More information about the ffmpeg-devel
mailing list