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 CF9674AE89 for ; Wed, 22 May 2024 10:37:50 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0C1A668D410; Wed, 22 May 2024 13:37:47 +0300 (EEST) Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2781368D222 for ; Wed, 22 May 2024 13:37:41 +0300 (EEST) Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-2e576057c2bso93936941fa.1 for ; Wed, 22 May 2024 03:37:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716374260; x=1716979060; 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=UUib4vlO3YQUrKT88AlMG3qUNgy4Ny9J1iVxwOPwcKo=; b=Xqvn6eqHj3HNki4zVw5ezCnug77p1Mx3YYCnsR3TvAC9xOPvbk8MIjvj7vlAhsymSw b0Wz3XFOuXh6VMHctuMnmd9l43qDVDglFlVOTnymdJldgWMXCTixoR3MlCugjZyOUdLF N9cX57Dfq6FZU1JBew2K8S13H/UHWGjwqoV6KtEPLdvJV3xKKxR5A97g14W9sbBPkjpw zGYWm8t3nfBpKL1S8uGMhfiAeAeHsrrExKcBI0BnWHYMj+vDQSFFa7pbhb0z18NIdfhh DXxcurg0CEfZr1hc8KglfMYImXth2YkZS6ECI877UNRienzIQ0MscZiKK7W5ZTk2eUON TMrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716374260; x=1716979060; 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=UUib4vlO3YQUrKT88AlMG3qUNgy4Ny9J1iVxwOPwcKo=; b=LA3owYW0EsD3D6P0AP/OipRPjyi+efdePiYuWYepfyAudq3DHzffOZm1q2dUOl2I49 4cb3MekloYjBdWHYKdls9hsPdyV/Ga4REw+MLQk5q3v9FJYwsP0F2XRdFPv671wBXK5y NtX41zY1kVIRisdM6iGpeRtRG8pO3280UNyozJOGyVZYY99n/DM7uYQwIhS+YOBfQNpL gbX+KQqYsONot3PH2lWcsdOuBiLduQSDzZkUIqr2V+rvhO2nVCBECGV/5Jli+zIMX/jR X9Uy4xk8WdnLu/J3LVfFJR+cONTnc3HV0rl8Of+8yB0lcagHx3XHlI8RNx+mqs6oYeLN /lWQ== X-Gm-Message-State: AOJu0YxIKOOY9gZ4InRzcN69Uk7d/CSxs9WNYVqT6no9FMUCNeisxxRY jP26eODAqZPr/H1GrrmRy6qCxVw1Mqdfz572lI7F0n4SJVoXp2vRFaJcQg== X-Google-Smtp-Source: AGHT+IGRwU3Z4XO1VIA+udQZ1ROkJ+8CFGD/+InZgKa1PCJ31ojIVBKVXOijXE9OS9W7uYMr1iL0ng== X-Received: by 2002:a05:651c:3d1:b0:2de:ca7f:c849 with SMTP id 38308e7fff4ca-2e94956389bmr11213661fa.43.1716374259673; Wed, 22 May 2024 03:37:39 -0700 (PDT) Received: from mariano (dynamic-adsl-84-220-189-10.clienti.tiscali.it. [84.220.189.10]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a5a69148b97sm1302614366b.114.2024.05.22.03.37.38 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 May 2024 03:37:39 -0700 (PDT) Received: by mariano (Postfix, from userid 1000) id DCD4FBFCE8; Wed, 22 May 2024 12:37:37 +0200 (CEST) Date: Wed, 22 May 2024 12:37:37 +0200 From: Stefano Sabatini To: FFmpeg development discussions and patches Message-ID: Mail-Followup-To: FFmpeg development discussions and patches References: <20240422155836.385333-1-ffmpeg-devel@pileofstuff.org> <20240422155836.385333-2-ffmpeg-devel@pileofstuff.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.1.4 (2021-12-11) Subject: Re: [FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means 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-05-05 22:04:36 +0100, Andrew Sayers wrote: > I'm still travelling, so the following thoughts might be a bit > half-formed. But I wanted to get some feedback before sitting down > for a proper think. [...] > > > I've also gone through the code looking for edge cases we haven't covered. > > > Here are some questions trying to prompt an "oh yeah I forgot to mention > > > that"-type answer. Anything where the answer is more like "that should > > > probably be rewritten to be clearer", let me know and I'll avoid confusing > > > newbies with it. > > > > > > > > av_ambient_viewing_environment_create_side_data() takes an AVFrame as its > > > first argument, and returns a new AVAmbientViewingEnvironment. What is the > > > context object for that function - AVFrame or AVAmbientViewingEnvironment? > > > > But this should be clear from the doxy: > > > > /** > > * Allocate and add an AVAmbientViewingEnvironment structure to an existing > > * AVFrame as side data. > > * > > * @return the newly allocated struct, or NULL on failure > > */ > > AVAmbientViewingEnvironment *av_ambient_viewing_environment_create_side_data(AVFrame *frame); > > I'm afraid it's not clear, at least to me. I think you're saying the > AVFrame is the context because a "create" function can't have a > context any more than a C++ "new" can be called as a member. But the > function's prefix points to the other conclusion, and neither signal > is clear enough on its own. No, what I'm saying is that in some cases you don't need to think in terms of contexts, in this case there is no context at all, the function takes a frame and modify it, and returns the ambient environment to be used by the following functions. This should be very clear by reading the doxy. There is no rule dictating the first param of each FFmpeg function should be a "context". > > My current thinking is to propose separate patches renaming arguments > to `ctx` whenever I find functions I can't parse. That's not as good > as a simple rule like "the first argument is always the context", but > better than adding a paragraph or two about how to read the docs. There cannot be such rule, because it would be false in many cases. > > Also, you are assuming that all the function should have a > > context. That's not the case, as you don't always need to keep track > > of a "context" when performing operations. > [...] > > > av_channel_description_bprint() takes a `struct AVBPrint *` as its first > > > argument, then `enum AVChannel`. Is the context AVBPrint, AVChannel, > > > or both? Does it make sense for a function to have two contexts? > > > > Again, this should be clear from the doxy: > > /** > > * Get a human readable string describing a given channel. > > * > > * @param buf pre-allocated buffer where to put the generated string > > * @param buf_size size in bytes of the buffer. > > * @param channel the AVChannel whose description to get > > * @return amount of bytes needed to hold the output string, or a negative AVERROR > > * on failure. If the returned value is bigger than buf_size, then the > > * string was truncated. > > */ > > int av_channel_description(char *buf, size_t buf_size, enum AVChannel channel); > > > > /** > > * bprint variant of av_channel_description(). > > * > > * @note the string will be appended to the bprint buffer. > > */ > > void av_channel_description_bprint(struct AVBPrint *bp, enum AVChannel channel_id); > > I think you're saying that I should look at which word appears more > often in the doxy ("channel") rather than which word appears first in > the argument list ("buf")? As above, the solution might be to rename > the variable in a separate patch rather than teach people another > special case. This is more about the semantics described in English language by the doxy (which is normative). Again, thinking in terms of "contexts" is misleading in this case. In this case you have two functions, av_channel_description writing a string to a buffer with fixed size, the second modifying an "AVBPrint" struct, which is a high-level buffer providing more flexibility (and "bprint" is used as a verb in the doxy, which might be misleading). Note that both signatures are mimicing the standard C library convention: memcpy(dst, dst_size, src) which in turn is a mnemonics for: dst = src meaning that we are copying data from src to dst. You might think that in fact you are operating on a context (the dst buffer or the AVBPrint struct), but you don't need to introduce the concept of context for these simple functions. _______________________________________________ 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".