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 0C71F461FF for ; Sun, 9 Jul 2023 09:48:22 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id AE75E68C331; Sun, 9 Jul 2023 12:48:19 +0300 (EEST) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D18D568C331 for ; Sun, 9 Jul 2023 12:48:12 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 8F1EE2404EC for ; Sun, 9 Jul 2023 11:48:12 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id n1NvV58lWiFi for ; Sun, 9 Jul 2023 11:48:11 +0200 (CEST) Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id E5DD32404EA for ; Sun, 9 Jul 2023 11:48:11 +0200 (CEST) Received: by lain.khirnov.net (Postfix, from userid 1000) id C42F71601B2; Sun, 9 Jul 2023 11:48:11 +0200 (CEST) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: <20230708190038.24324-1-jamrial@gmail.com> References: <20230708190038.24324-1-jamrial@gmail.com> Mail-Followup-To: FFmpeg development discussions and patches Date: Sun, 09 Jul 2023 11:48:11 +0200 Message-ID: <168889609177.542.15252833672314979069@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Quoting James Almer (2023-07-08 21:00:36) > -/* > - * The core of the receive_frame_wrapper for the decoders implementing > - * the simple API. Certain decoders might consume partial packets without > - * returning any output, so this function needs to be called in a loop until it > - * returns EAGAIN. > - **/ > -static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples) > +static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples) > { > - AVCodecInternal *avci = avctx->internal; > - AVPacket *const pkt = avci->in_pkt; > - const FFCodec *const codec = ffcodec(avctx->codec); > - int got_frame, actual_got_frame; > - int ret; > - > - if (!pkt->data && !avci->draining) { > - av_packet_unref(pkt); > - ret = ff_decode_get_packet(avctx, pkt); > - if (ret < 0 && ret != AVERROR_EOF) > - return ret; > - } > - > - // Some codecs (at least wma lossless) will crash when feeding drain packets > - // after EOF was signaled. > - if (avci->draining_done) > - return AVERROR_EOF; > - > - if (!pkt->data && > - !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY || > - avctx->active_thread_type & FF_THREAD_FRAME)) > - return AVERROR_EOF; > - > - got_frame = 0; > - > - if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) { > - ret = ff_thread_decode_frame(avctx, frame, &got_frame, pkt); > - } else { > - ret = codec->cb.decode(avctx, frame, &got_frame, pkt); > - > - if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS)) > - frame->pkt_dts = pkt->dts; > - if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) { > -#if FF_API_FRAME_PKT > -FF_DISABLE_DEPRECATION_WARNINGS > - if(!avctx->has_b_frames) > - frame->pkt_pos = pkt->pos; > -FF_ENABLE_DEPRECATION_WARNINGS > -#endif > - //FIXME these should be under if(!avctx->has_b_frames) > - /* get_buffer is supposed to set frame parameters */ > - if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) { > - if (!frame->sample_aspect_ratio.num) frame->sample_aspect_ratio = avctx->sample_aspect_ratio; > - if (!frame->width) frame->width = avctx->width; > - if (!frame->height) frame->height = avctx->height; > - if (frame->format == AV_PIX_FMT_NONE) frame->format = avctx->pix_fmt; > - } > - } > - } > - emms_c(); > - actual_got_frame = got_frame; > + AVCodecInternal *avci = avctx->internal; > + int ret = 0; > > if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) { > if (frame->flags & AV_FRAME_FLAG_DISCARD) > - got_frame = 0; > + ret = AVERROR(EAGAIN); It's quite strange an unexpected for a function called discard_samples() to do anything with video. By leaving video processing in the caller you also save a level of indentation. > } else if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) { > uint8_t *side; > size_t side_size; > @@ -359,16 +304,10 @@ FF_ENABLE_DEPRECATION_WARNINGS > uint8_t skip_reason = 0; > uint8_t discard_reason = 0; > > - if (ret >= 0 && got_frame) { > if (frame->format == AV_SAMPLE_FMT_NONE) > frame->format = avctx->sample_fmt; > - if (!frame->ch_layout.nb_channels) { > - int ret2 = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout); > - if (ret2 < 0) { > - ret = ret2; > - got_frame = 0; > - } > - } > + if (!frame->ch_layout.nb_channels) > + ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout); > #if FF_API_OLD_CHANNEL_LAYOUT > FF_DISABLE_DEPRECATION_WARNINGS > if (!frame->channel_layout) > @@ -380,7 +319,6 @@ FF_ENABLE_DEPRECATION_WARNINGS > #endif > if (!frame->sample_rate) > frame->sample_rate = avctx->sample_rate; > - } So the rest of the code below was previously executed even when there is no frame? The change seems correct, but not I wouldn't expect it in a 'move' commit, so perhaps mention it in the commit message. > > side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size); > if(side && side_size>=10) { > @@ -393,21 +331,21 @@ FF_ENABLE_DEPRECATION_WARNINGS > discard_reason = AV_RL8(side + 9); > } > > - if ((frame->flags & AV_FRAME_FLAG_DISCARD) && got_frame && > + if ((frame->flags & AV_FRAME_FLAG_DISCARD) && !ret && > !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) { > avci->skip_samples = FFMAX(0, avci->skip_samples - frame->nb_samples); > - got_frame = 0; > *discarded_samples += frame->nb_samples; > + ret = AVERROR(EAGAIN); If I'm reading correctly, all the blocks below are conditioned on !ret, so you might as well return here and drop those checks. -- Anton Khirnov _______________________________________________ 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".