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 34BD647772 for ; Sun, 21 Jan 2024 10:30:41 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DED0D68CEC1; Sun, 21 Jan 2024 12:30:38 +0200 (EET) Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E43D168CAAC for ; Sun, 21 Jan 2024 12:30:31 +0200 (EET) Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-554fe147ddeso3056781a12.3 for ; Sun, 21 Jan 2024 02:30:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705833030; x=1706437830; darn=ffmpeg.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:from:to:cc:subject :date:message-id:reply-to; bh=iALkvuxvSLeykoT11AHlA+h1O5WLxmnQ4iuOPqRms6E=; b=R/pf2/HHf3kriKKJTFXQm/e7545R85TzTixa9x70+KacfrrLmRChfCqFZ0DA/S888j 2hnSXq40rF3tOQaiJ8CHwfG6fPTau9WN2vzxbR8Ho0oqIlmZaLHIOHil95RSH6WIDhhW jcVbqShlzyzzHkw2x04cZQOtkd0fUbBCmTwD7GHDWJfKH9N22X2jm2tkIDgQCNU6yi+9 tT8E3J7uegflUwcJmXqHRYNOq3vdpNx/i2K8XHXG6SzOOCnNWXltZVgvyld7PN1VN9qv +MWrRJZ8qLuWzE0hQpnIkUhPLge/sWXdA5B0MqW7rLLvIAaI85CwFkt8Iif9Iynnc2Ph PebQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705833030; x=1706437830; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=iALkvuxvSLeykoT11AHlA+h1O5WLxmnQ4iuOPqRms6E=; b=W0+d8NADl1PaUfoiGi//qZXd8VewOHQrHsS9O56MYOoQ/RpkyrZxrABaF+D63Po3F3 wi3C1wCcQZ8ynOzHrykIxSAPqJSp1HFOy8bFu+xMR+m1Qpbgo9Kbd6UvLjEhSiSrMXPQ 3WNPyarGFdFoPwExZw2TvEpwoHgksoZQ7eAZIhq18IFby8NYjzRLOawL/CI+zp4+ssaD Ax1jyPW80CG/n1zrUjCxHzDW994WpCUudg5ZO3iLNi/XXlEuI36vwSiAUf9akY8VK+rr E1+R8dlEwjCzQve+Ns1XsDzebQTzOKTsteaFVsktgh+z6GQMGO9h1EVik7kRK3Y4tPFh J69A== X-Gm-Message-State: AOJu0YwrL1i9t10lC28o5BXxmiJw2n0i7rmrM+gXITBbUmwNUG6Zogp6 tGp3cxY5TVZ1Inn1Sp3e8ap7QUr7D/fhulqIkkMnGP188eSFoAtBxFshV+DF X-Google-Smtp-Source: AGHT+IHps/QSLmjXTOh9RFTv5gIksj67Wedfnx8GvcR41HZUlip9su9kSFjddkOjR4D6DAj/mkHdlw== X-Received: by 2002:a17:907:d384:b0:a2f:c0cb:3d4a with SMTP id vh4-20020a170907d38400b00a2fc0cb3d4amr818714ejc.284.1705833029975; Sun, 21 Jan 2024 02:30:29 -0800 (PST) Received: from mariano (dynamic-adsl-84-220-189-10.clienti.tiscali.it. [84.220.189.10]) by smtp.gmail.com with ESMTPSA id o19-20020a17090608d300b00a2adc93e308sm12161959eje.222.2024.01.21.02.30.29 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 21 Jan 2024 02:30:29 -0800 (PST) Received: by mariano (Postfix, from userid 1000) id AEA11BFCDC; Sun, 21 Jan 2024 11:30:27 +0100 (CET) Date: Sun, 21 Jan 2024 11:30:27 +0100 From: Stefano Sabatini To: FFmpeg development discussions and patches Message-ID: Mail-Followup-To: FFmpeg development discussions and patches References: <20240120152408.606235-1-stefasab@gmail.com> <170581900840.8914.5686749055175923589@lain.khirnov.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <170581900840.8914.5686749055175923589@lain.khirnov.net> User-Agent: Mutt/2.1.4 (2021-12-11) 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 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: On date Sunday 2024-01-21 07:36:48 +0100, Anton Khirnov wrote: > 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. fixed locally > > - 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. Yes, but this would complicate the logic for small gain. > > > } > > > > /* 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. Dropped. > > > + 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. Yes, probably it's better to drop this entirely since we have more puntual reporting now (and "exactly two stream" is wrong) > > > - " 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. 44kHz != 44100 I could use 44.1 but this is not the unit used when setting the option, so better to be explicit. Thanks. _______________________________________________ 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".