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 9A645405BC for ; Wed, 27 Apr 2022 08:51:28 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 75CAB68B3B3; Wed, 27 Apr 2022 11:51:26 +0300 (EEST) Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 71DF368B38C for ; Wed, 27 Apr 2022 11:51:18 +0300 (EEST) Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 23R8pE0v016944-23R8pE0w016944; Wed, 27 Apr 2022 11:51:14 +0300 Received: from foo.martin.st (host-97-187.parnet.fi [77.234.97.187]) by mail9.parnet.fi (Postfix) with ESMTPS id 8EE71A142C; Wed, 27 Apr 2022 11:51:14 +0300 (EEST) Date: Wed, 27 Apr 2022 11:51:13 +0300 (EEST) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: FFmpeg development discussions and patches In-Reply-To: <919e3498-f841-e879-580-18237036e056@martin.st> Message-ID: <37b2b612-536e-f85e-beb3-85bb29544b1@martin.st> References: <20220218125319.243373-1-asn@cryptomilk.org> <919e3498-f841-e879-580-18237036e056@martin.st> MIME-Version: 1.0 X-FE-Policy-ID: 3:14:2:SYSTEM X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/openh264: return (DE|EN)CODER_NOT_FOUND if version check fails 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 Cc: Andreas Schneider Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-15"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Wed, 20 Apr 2022, Martin Storsj=F6 wrote: > On Fri, 18 Feb 2022, Andreas Schneider wrote: > >> Signed-off-by: Andreas Schneider >> --- >> libavcodec/libopenh264dec.c | 2 +- >> libavcodec/libopenh264enc.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> = >> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c >> index 7f5e85402a..97d3630df6 100644 >> --- a/libavcodec/libopenh264dec.c >> +++ b/libavcodec/libopenh264dec.c >> @@ -56,7 +56,7 @@ static av_cold int svc_decode_init(AVCodecContext *avc= tx) >> WelsTraceCallback callback_function; >> >> if ((err =3D ff_libopenh264_check_version(avctx)) < 0) >> - return err; >> + return AVERROR_DECODER_NOT_FOUND; >> >> if (WelsCreateDecoder(&s->decoder)) { >> av_log(avctx, AV_LOG_ERROR, "Unable to create decoder\n"); >> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c >> index 7c0501a2eb..7649e7b025 100644 >> --- a/libavcodec/libopenh264enc.c >> +++ b/libavcodec/libopenh264enc.c >> @@ -137,7 +137,7 @@ static av_cold int svc_encode_init(AVCodecContext = >> *avctx) >> AVCPBProperties *props; >> >> if ((err =3D ff_libopenh264_check_version(avctx)) < 0) >> - return err; >> + return AVERROR_ENCODER_NOT_FOUND; >> >> if (WelsCreateSVCEncoder(&s->encoder)) { >> av_log(avctx, AV_LOG_ERROR, "Unable to create encoder\n"); >> -- = >> 2.35.1 > > This looks reasonable to me, so I could push this in a little while if = > there's no more comments on it. > > But the patch lacks an explanation of _why_ this is done, in addition to = > _what_ it does. I presume that's because the current error code makes som= e = > decoder/encoder selection logic error out entirely, instead of continuing = > trying some other codec - is that right? That would really be valuable to = > include in the commit message. Actually, I don't see anything in the code where returning = AVERROR_DECODER_NOT_FOUND would behave differently than returning = AVERROR(EINVAL) (as I would say it does right now). So what's the motive for this patch, what does it change in practice? I guess it changes what error message is printed (which certainly makes = sense to change), but it would be very relevant to know whether this is a = cosmetic change or if it changes something functionally that I haven't = found. // Martin _______________________________________________ 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".