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 E263144D6B for ; Sun, 21 Jan 2024 06:36:59 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6314268CF9C; Sun, 21 Jan 2024 08:36:56 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 58ED968CD89 for ; Sun, 21 Jan 2024 08:36:49 +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=nPqcDbB4; dkim-atps=neutral Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 1A3D82405F2; Sun, 21 Jan 2024 07:36:49 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavis, port 10024) with ESMTP id B5ZH8WhRFmUF; Sun, 21 Jan 2024 07:36:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=khirnov.net; s=mail; t=1705819008; bh=jdiku+qe1TmYKkq+kEEQap/TysAvnJIAr8008FOqnV8=; h=Subject:From:To:Cc:In-Reply-To:References:Date:From; b=nPqcDbB4l1rP6GjcCLhyRK1Uf25n5URjYIKPeKBVMqkFY67NKY9+FECbMsHKAcM5f q+TOXhXSoyp/gkkBXxaAIHG1KGVTy0cTisWkFvvI46WEkWKuMhqjqz0AZWVuN2yumm pAMxyJkGzxwOnWAN9jncaXZGRs+og7UFArTtWSORYMgZvIHWot/0716R4OWmpV4AS0 IyZgX4Z2L16hX1I35Mba8VGULEv3HbOnOBBNIqcu3cJWA+R8Drll5NUEol2nXtNJjt iiTA6AwbqQWBrRgkZyNqcNZkHv/uxAE1O13Ms2ug6CVwBS5clUwS2J0uHnLEIE8s+5 7zwQ1tcd8PdRQ== 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 9245A2404E5; Sun, 21 Jan 2024 07:36:48 +0100 (CET) Received: by lain.khirnov.net (Postfix, from userid 1000) id 699C61601B9; Sun, 21 Jan 2024 07:36:48 +0100 (CET) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: <20240120152408.606235-1-stefasab@gmail.com> References: <20240120152408.606235-1-stefasab@gmail.com> Mail-Followup-To: FFmpeg development discussions and patches , Stefano Sabatini Date: Sun, 21 Jan 2024 07:36:48 +0100 Message-ID: <170581900840.8914.5686749055175923589@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging 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: Stefano Sabatini 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 Stefano Sabatini (2024-01-20 16:24:07) > if ((c->sys->time_base.den != 25 && c->sys->time_base.den != 50) || c->sys->time_base.num != 1) { > - if (c->ast[0] && c->ast[0]->codecpar->sample_rate != 48000) > - goto bail_out; > - if (c->ast[1] && c->ast[1]->codecpar->sample_rate != 48000) > - goto bail_out; > + int j; No need to declare a loop variable outside of the loop. Also, there's already i. > - if (((c->n_ast > 1) && (c->sys->n_difchan < 2)) || > - ((c->n_ast > 2) && (c->sys->n_difchan < 4))) { > - /* only 2 stereo pairs allowed in 50Mbps mode */ > - goto bail_out; > + if ((c->n_ast > 1) && (c->sys->n_difchan < 2)) { > + av_log(s, AV_LOG_ERROR, > + "Invalid number of channels %d, only 1 stereo pairs is allowed in 25Mps mode.\n", > + c->n_ast); > + return AVERROR_INVALIDDATA; > + } > + if ((c->n_ast > 2) && (c->sys->n_difchan < 4)) { > + av_log(s, AV_LOG_ERROR, > + "Invalid number of channels %d, only 2 stereo pairs are allowed in 50Mps mode.\n", > + c->n_ast); > + return AVERROR_INVALIDDATA; Surely this can be done in one log statement. > } > > /* Ok, everything seems to be in working order */ > @@ -376,14 +427,14 @@ static DVMuxContext* dv_init_mux(AVFormatContext* s) > if (!c->ast[i]) > continue; > c->audio_data[i] = av_fifo_alloc2(100 * MAX_AUDIO_FRAME_SIZE, 1, 0); > - if (!c->audio_data[i]) > - goto bail_out; > + if (!c->audio_data[i]) { > + av_log(s, AV_LOG_ERROR, > + "Failed to allocate internal buffer.\n"); Dedicated log messages for small malloc failures are useless bloat. > + return AVERROR(ENOMEM); > + } > } > > - return c; > - > -bail_out: > - return NULL; > + return 0; > } > > static int dv_write_header(AVFormatContext *s) > @@ -392,10 +443,10 @@ static int dv_write_header(AVFormatContext *s) > DVMuxContext *dvc = s->priv_data; > AVDictionaryEntry *tcr = av_dict_get(s->metadata, "timecode", NULL, 0); > > - if (!dv_init_mux(s)) { > + if (dv_init_mux(s) < 0) { > av_log(s, AV_LOG_ERROR, "Can't initialize DV format!\n" > "Make sure that you supply exactly two streams:\n" This seems inconsistent with the other checks. > - " video: 25fps or 29.97fps, audio: 2ch/48|44|32kHz/PCM\n" > + " video: 25fps or 29.97fps, audio: 2ch/48000|44100|32000Hz/PCM\n" This does not seem like an improvement. -- 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".