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 4E1C448005 for ; Thu, 7 Mar 2024 12:25:04 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6A6BE68CF40; Thu, 7 Mar 2024 14:25:02 +0200 (EET) Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.182]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0911A68CCD4 for ; Thu, 7 Mar 2024 14:24:55 +0200 (EET) Received: by mail-pf1-f182.google.com with SMTP id d2e1a72fcca58-6e6381df003so752990b3a.0 for ; Thu, 07 Mar 2024 04:24:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709814292; x=1710419092; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=XoBx1aF9wjPu0tTuiVj5VSbqkDyZBCqB5U15skbvdRA=; b=EnWFrF8mbiwcVJN9s4zod8wWuGZT7qpq1O2znt4zXDYmLekAZGN6QlriOB3CKAEYuj ZJXXOQxE/JF0+BVjUL7JDEorkMc1MXc82GGchPWyJFtzDt7GQER7Yz7pG9XeNG6k1azl rcEZ8zt2bPOESqb/DOMSL7xGcojkMN/US0GKvdhfx9++/QR2j8mHZlfEHQ7BWHbXUFEA uRan1N73XPn9pfMInQ7xRM7eifYh/LJ6y4YnyeBGrxn3lNYCy5UVGsUPU2LsANf79r02 1HcC7EavFs2VPpSqk7Khoy7hb1DybjuVrXfHSIhtcwEnNb/DRKfNl7ob/2msTcIvG6ll le4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709814292; x=1710419092; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=XoBx1aF9wjPu0tTuiVj5VSbqkDyZBCqB5U15skbvdRA=; b=XOt/CzAs9L5VijKAVDGg3sZ0QI1wsjlsyMNN6bwqieIQP3nXwaKZkfSnPlF6y+z1FJ +4nXaw0+gEJdAejyr83SFdC3RyJ77pY5NDmrJXFOlfkKgbU0BZHgiVOJMEKa2qGPpkBs mUY0qQrCZH+oNnI3hhSsuC62leeQVStnwBktnfWdyCWXRkhHOrXtTLmedH3QP6WBkV4x 4WOrzm3JmwAEW2q3gmu0ls3zBlaZju7cwWsRIFNdZA7mH6ztZPjKkwniP8VjaKGQQeeN pTiPW08IzKkqM9SFbZQlu2WAnVniTsICTHFi9L8jve3OSLKeP5v9KSKrMg6qgNUmTbZ+ MEnQ== X-Gm-Message-State: AOJu0YyWh2t3CJfUiwzcqnmhiNyHG0rMPAGCHTetYS9oYEG0r16sSXif j3BNaB4dCHmX0yrw11SJGd8WFuYNBlehyfsOlfciRkWveMuq1/d76Hv8lPwR X-Google-Smtp-Source: AGHT+IFfMVioPfaunP6z5m+t3FqThP5qATJGtkhEQfgq1nuzAN1j9iK6NikbRgL/4tlfICjTTwt21w== X-Received: by 2002:a05:6a21:999d:b0:1a1:5144:79ab with SMTP id ve29-20020a056a21999d00b001a1514479abmr8018531pzb.10.1709814292031; Thu, 07 Mar 2024 04:24:52 -0800 (PST) Received: from [192.168.0.14] ([190.194.169.124]) by smtp.gmail.com with ESMTPSA id p13-20020a635b0d000000b005c6e8fa9f24sm12659833pgb.49.2024.03.07.04.24.50 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Mar 2024 04:24:51 -0800 (PST) Message-ID: <264761ea-8e47-46f0-b70e-d5fd6b6605d3@gmail.com> Date: Thu, 7 Mar 2024 09:25:07 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240304130657.30631-1-anton@khirnov.net> <20240304130657.30631-23-anton@khirnov.net> <170963280234.29002.12316451033069413004@lain.khirnov.net> <170981392457.662.13858277976495309267@lain.khirnov.net> Content-Language: en-US From: James Almer In-Reply-To: <170981392457.662.13858277976495309267@lain.khirnov.net> 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-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/7/2024 9:18 AM, Anton Khirnov wrote: > 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. I wouldn't. It adds an extra layer of abstraction for no real gain. And the name in this case is very specific and self explanatory in how it's different from the alternative. _______________________________________________ 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".