From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 42BD845032 for ; Wed, 29 Mar 2023 12:17:03 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 942CF68C40D; Wed, 29 Mar 2023 15:16:59 +0300 (EEST) Received: from mail-oa1-f48.google.com (mail-oa1-f48.google.com [209.85.160.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 96E5268C35A for ; Wed, 29 Mar 2023 15:16:53 +0300 (EEST) Received: by mail-oa1-f48.google.com with SMTP id 586e51a60fabf-177b78067ffso15981001fac.7 for ; Wed, 29 Mar 2023 05:16:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680092211; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=Wcqh44LluTIoKKp+ZiT4wskRRheUNc38vAVpOjZ887I=; b=USd2/I2OSz3pcJEAB7xVfiy5y+z2hN+pu0I+LvUbm5zJh0vis0SRdu/tZ9tJ2o9RaX 7lMsdd+DfEZWOiaOhfgHSEYOymb2gLyvvwjnM9EszUk2F6ECNi1dDrr9MwHxr0/ckb0U nlb/3HK5qZ1zN2CxOnkHH133YPOMxFXnkJwb0tAxK5HettcjnIPTZHd4DEn40BTdY1Kh jc+A9vmBkpkKURVEarPiQiE73i2M101f9YmCGVvDT1448vmUfeCrH9iOcWE0pSL/oJ6J 2eQdDQd+c7IvxpD0A20FtGu3RxuE806LVDCKl+4QBSf5KhZgtXlmigWFFFFQy2mDM+WL yI/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680092211; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Wcqh44LluTIoKKp+ZiT4wskRRheUNc38vAVpOjZ887I=; b=jbL9b0JKKq9sFTbbfF5QZyiuPC6wUnp6pjHeavUX4+Iwz6u8nNsC7tDRIU9RD8z4R2 DNp0HGqtIgdVfCWCjk0nwtOkgyHED8CtprwTZ2cVMO2fQ85fOA0S6PZ6D04zB9IarH98 YbEL1JAxZmN+5nSlgNlKMV1u288BnTP/Vou9TKoTqKVwKxni21dwmPjsIxBlLPonEC3n dVdFLe6YtjhS8TDsu2V3Znq/UV8zLTlYGLa3WIEKy9XbQrmL94+COoJ39OyqjUG2SzG3 mznrWUiP3+7nOyI91wqmn2pNoFbv7p/Lys9CU0dseORi+H2uR192A7q6FBmA9xtNwDJA BOqg== X-Gm-Message-State: AAQBX9c67T+lo4knmh0HZQFCaHPGF2uzacmkbzM+k41/Z1hF+81kncQU xDab4RpMDcLi0OO4qDbW9X4ESCXdKog= X-Google-Smtp-Source: AKy350aSDQh/reMYkgy4ZuWFOphTq6bH4RU0FXVCGyLySvypqBCG2xKm8J3Tm++4ZSbFTTsuz/LcPQ== X-Received: by 2002:a05:6870:42c8:b0:176:ddbd:56ee with SMTP id z8-20020a05687042c800b00176ddbd56eemr1008465oah.28.1680092211573; Wed, 29 Mar 2023 05:16:51 -0700 (PDT) Received: from [192.168.0.14] (host197.190-225-105.telecom.net.ar. [190.225.105.197]) by smtp.gmail.com with ESMTPSA id vl9-20020a0568710e8900b0017ad7a5f57dsm11726772oab.11.2023.03.29.05.16.50 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Mar 2023 05:16:51 -0700 (PDT) Message-ID: <62f45ea9-aa27-e1dd-a8c0-f3cc80f32ff0@gmail.com> Date: Wed, 29 Mar 2023 09:16:49 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20230328135007.1765-1-d.kozinski@samsung.com> From: James Almer In-Reply-To: <20230328135007.1765-1-d.kozinski@samsung.com> Subject: Re: [FFmpeg-devel] [PATCH v18 06/10] avcodec/evc_decoder: Provided support for EVC decoder X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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; > +} _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".