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 E783349209 for ; Thu, 7 Mar 2024 12:19:01 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C0BE668CCAA; Thu, 7 Mar 2024 14:18:58 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3169768CA4C for ; Thu, 7 Mar 2024 14:18:52 +0200 (EET) Authentication-Results: mail0.khirnov.net; dkim=pass (2048-bit key; unprotected) header.d=khirnov.net header.i=@khirnov.net header.a=rsa-sha256 header.s=mail header.b=XXdIvb1q; dkim-atps=neutral Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id B9A3B24048D for ; Thu, 7 Mar 2024 13:18:51 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavis, port 10024) with ESMTP id JOWisbWbya_p for ; Thu, 7 Mar 2024 13:18:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=khirnov.net; s=mail; t=1709813930; bh=kk9DG3Emg42aNvW/GvKog/3LlJhEkT+9HtvU4F3wIAE=; h=Subject:From:To:In-Reply-To:References:Date:From; b=XXdIvb1q8Uc2CPHWRRKib989jO/EJ8I88xYqLYW80UMt0znn0SkKDX65oC8DHjGu5 GffcugKUN1gfkx8cj7i4yfkOkYsIeAE6uU4tbWYZa5TidnN/k8IHZC6MPlZEuM+FAX 0lskT90XWtWc7/mS8C6F/Bh5Qru6KnZVI5n+ofGD2RpQ7L8Qw4oS0iIk0hmcfsEbeC MXXCrh5wHOpB5xh15Dc5j+jc7rLj2+44DEvOxErbYo9Qt6CbThSuqu5FzVf0ED5tcv Hsy3djjsMfydrG0KfVPTwgdpiyxSN2NwLlAEX392xGIbj3Gd/tLwSPw0iV9eXUFYUF x95Z/87srP2Ig== 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 B2DA1240177 for ; Thu, 7 Mar 2024 13:18:50 +0100 (CET) Received: by lain.khirnov.net (Postfix, from userid 1000) id 923BA1601B9; Thu, 7 Mar 2024 13:18:44 +0100 (CET) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: References: <20240304130657.30631-1-anton@khirnov.net> <20240304130657.30631-23-anton@khirnov.net> <170963280234.29002.12316451033069413004@lain.khirnov.net> Mail-Followup-To: FFmpeg development discussions and patches Date: Thu, 07 Mar 2024 13:18:44 +0100 Message-ID: <170981392457.662.13858277976495309267@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 23/29] avcodec/mpeg12dec: use ff_frame_new_side_data 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 Andreas Rheinhardt (2024-03-07 12:19:28) > Anton Khirnov: > > Quoting Andreas Rheinhardt (2024-03-04 14:36:09) > >> Anton Khirnov: > >>> From: Niklas Haas > >>> > >>> For consistency, even though this cannot be overriden at the packet > >>> level. > >>> --- > >>> libavcodec/mpeg12dec.c | 18 ++++++++++-------- > >>> 1 file changed, 10 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c > >>> index 3a2f17e508..aa116336dd 100644 > >>> --- a/libavcodec/mpeg12dec.c > >>> +++ b/libavcodec/mpeg12dec.c > >>> @@ -2531,15 +2531,17 @@ static int mpeg_decode_frame(AVCodecContext *avctx, AVFrame *picture, > >>> > >>> if (s->timecode_frame_start != -1 && *got_output) { > >>> char tcbuf[AV_TIMECODE_STR_SIZE]; > >>> - AVFrameSideData *tcside = av_frame_new_side_data(picture, > >>> - AV_FRAME_DATA_GOP_TIMECODE, > >>> - sizeof(int64_t)); > >>> - if (!tcside) > >>> - return AVERROR(ENOMEM); > >>> - memcpy(tcside->data, &s->timecode_frame_start, sizeof(int64_t)); > >>> + AVFrameSideData *tcside; > >>> + ret = ff_frame_new_side_data(avctx, picture, AV_FRAME_DATA_GOP_TIMECODE, > >>> + sizeof(int64_t), &tcside); > >>> + if (ret < 0) > >>> + return ret; > >>> + if (tcside) { > >>> + memcpy(tcside->data, &s->timecode_frame_start, sizeof(int64_t)); > >>> > >>> - av_timecode_make_mpeg_tc_string(tcbuf, s->timecode_frame_start); > >>> - av_dict_set(&picture->metadata, "timecode", tcbuf, 0); > >>> + av_timecode_make_mpeg_tc_string(tcbuf, s->timecode_frame_start); > >>> + av_dict_set(&picture->metadata, "timecode", tcbuf, 0); > >>> + } > >>> > >>> s->timecode_frame_start = -1; > >>> } > >> > >> -1 to everything that is only done for consistency. > > > > I prefer consistency here, otherwise the decoder authors have to choose > > which function to use, and they are often not aware of the precise > > implications of thise choice. Better to always use just one function. > > > > It adds unnecessary checks and given that internal API is updated more > frequently it is likely to lead to unnecessary further changes lateron. > Furthermore, mjpeg still emits an allocation failure error message > without knowing whether it was really allocation failure. "Could not allocate frame side data" seems appropriate to me, it really is what happened, whatever the actual reason is. > Finally, if we really believed decoder authors to be that uninformed, we > should remove ff_get_buffer() from decoders altogether and only use the > ff_thread_get_buffer() wrapper. I'd be in favor, fewer functions is better. -- 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".