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 4581048D2A for ; Wed, 24 Apr 2024 10:39:17 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C327268D2A1; Wed, 24 Apr 2024 13:39:14 +0300 (EEST) Received: from mail-lf1-f45.google.com (mail-lf1-f45.google.com [209.85.167.45]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 01DD668D197 for ; Wed, 24 Apr 2024 13:39:07 +0300 (EEST) Received: by mail-lf1-f45.google.com with SMTP id 2adb3069b0e04-516d6c1e238so8450123e87.2 for ; Wed, 24 Apr 2024 03:39:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=martin-st.20230601.gappssmtp.com; s=20230601; t=1713955146; x=1714559946; darn=ffmpeg.org; h=mime-version:references:message-id:in-reply-to:subject:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=URhTomT1TplllQ3wyaBKT46zmS2PhJHNTHnq1TctxZY=; b=Y4u6CJ2WwQHv5maMbyUjZser+na5LIQsDON4UKvp27F+54s5vOsc1fEFv4ZMkRZ7IC Nl5ngQ12+oiAu6YHUSBZtZ/GA1bmewELBpM1Dd1ELUkOEh/IRDmxUVTCE4pV4JMeliwX +1ZsIJBclDfzBh/e+n/g6SJjZ7OdBeWIemdT3i0gcDHzbLtIeSNUXq48AITuddNm6ppm nLb7I6usOlw5V2h7Qe+yEHsbualiXS0lxLG9eFwU6dAoFsjKXfgw09m9oyoolGoSGPAq skg0VOePPzohaUVJVru5Y1gwy7XzC6O1Q+fgsy4dldFkYtCD9T0WrLHti9RXK7dHyW51 kgnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713955146; x=1714559946; h=mime-version:references:message-id:in-reply-to:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=URhTomT1TplllQ3wyaBKT46zmS2PhJHNTHnq1TctxZY=; b=KDaiLA1MDKcwKV9B5/EG5O4L6YFym/PgpWB21RrwHmZMFlix2rmSLdZpqtWKp/NFo0 pgjfDS6wpx3qOlVYqLsKvDWusojiW0p4dGVU91fjEySrPjthJjgPHhX3XsvDvUKoihhY yDVYekow7Oq+OvjwyEfih0npikJHsGPFZUUqUf92Kd2Z6l3Lmj78ayEf8E25gyPMcvfF veMnbRuu2IuxAXVEQwvm8V4KHYRCKTmtJtLVL514CKfMz+Rcv+fmNgFKYFnyitmTf8be Jh/+qoa6cwivUMR5fkXh1N8LTnY4zhi2OYU4Q1bgAqCmNIYwG6YvDOT0Xva0mJZqW1EB wQqQ== X-Gm-Message-State: AOJu0YyjwTfM0wZLtIVt+eJJm54sF1dhzWSgriYaGteR429gMD1t3IC7 dIuCj2jmzBfBqOcleQnFbLZcz+Wx/9jz9odC1DYBQ4s07H3cLHLebOLd6g51QMnoJ02XdawA04v 6RQ== X-Google-Smtp-Source: AGHT+IGrgtIEoNk8QjJN+kSmpAXqQF2vs3DCwYIOYl3Hk9Iqe/7t6N0lOpnUObgdw553FG//J210fA== X-Received: by 2002:a05:6512:38c9:b0:51b:e7d7:2eb9 with SMTP id p9-20020a05651238c900b0051be7d72eb9mr837841lft.28.1713955146135; Wed, 24 Apr 2024 03:39:06 -0700 (PDT) Received: from tunnel335574-pt.tunnel.tserv24.sto1.ipv6.he.net (tunnel335574-pt.tunnel.tserv24.sto1.ipv6.he.net. [2001:470:27:11::2]) by smtp.gmail.com with ESMTPSA id fc19-20020a056512139300b0051bbb93e7c3sm421535lfb.194.2024.04.24.03.39.05 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Apr 2024 03:39:05 -0700 (PDT) Date: Wed, 24 Apr 2024 13:39:02 +0300 (EEST) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: FFmpeg development discussions and patches In-Reply-To: Message-ID: <6f6ed3eb-71e-b770-7171-99616e289f@martin.st> References: MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH] avdevice/avfoundation: fix macOS/iOS/tvOS SDK conditional checks 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 Wed, 17 Apr 2024, Marvin Scholz wrote: > This fixes the checks to properly use runtime feature detection and > check the SDK version (*_MAX_ALLOWED) instead of the targeted version > for the relevant APIs. As these things are pretty hard to think straight about, it could be good with a more concrete example of what this achieves. I.e. if building with -mmacosx-version-min=10.13, we can still use the macOS 10.15 specific APIs, if they were available at build time, via the runtime check. > > The target is still checked (*_MIN_REQUIRED) to avoid using deprecated > methods when targeting new enough versions. > --- > libavdevice/avfoundation.m | 164 ++++++++++++++++++++++++++----------- > 1 file changed, 116 insertions(+), 48 deletions(-) The diff is pretty hard to read as is, but when applied and viewed with "git show -w", it becomes clearer. The changes from TARGET_OS_IPHONE to TARGET_OS_IOS is pretty subtle, iirc TARGET_OS_IPHONE was any non-desktop platform (ios/tvos/watchos etc), while TARGET_OS_IOS specifically is iOS. The change looks right, but it might be good to spell this out as well. Specifically also, that TARGET_OS_IPHONE covers a whole class of OSes, while TARGET_OS_IOS is one OS - but the version defines for that OS are __IPHONE_OS_VERSION_MIN_REQUIRED and __IPHONE_OS_VERSION_MAX_ALLOWED. > + /* If the targeted macOS is new enough, this fallback case can never be reached, so do not > + * use a deprecated API to avoid compiler warnings. > + */ This sentence gets somewhat warped up at some point, so I don't think it exactly means and is understandable as you meant it. What about this: If the targeted macOS is new enough, use of older APIs will cause deprecation warnings. Due to the availability check, we actually won't ever execute the code in such builds, but the compiler will still warn about it, unless we actually ifdef out the reference. Outside of what the patch does, I see the existing file uses this construct in a few places: #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070 I think it would seem more consistent to update this to use TARGET_OS_OSX instead of negating TARGET_OS_IPHONE - or is there something I'm missing? As for alternative ways of doing this, that would be less unwieldy - I have something like this in mind: #define SDK_AT_LEAST(macos, ios, tvos) \ (TARGET_OS_OSX && MAC_OS_X_VERSION_MAX_ALLOWED >= macos) || \ (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= ios) || \ (TARGET_OS_TV && __TV_OS_VERSION_MAX_ALLOWED >= tvos) #if SDK_AT_LEAST(__MAC_10_15, __IPHONE_10_0, __TVOS_17_0) We could add similar macros for both SDK_AT_LEAST and TARGET_VERSION_AT_LEAST, and variants for different combinations of macos/ios/tvos for when we don't want to specify all of them. We can't use defined(macos) etc within this context though, so if we want to go this way, we'd need to start out with ifdefs for all the defines we use, like this: #ifndef __MAC_10_15 #define __MAC_10_15 #endif There's of course a bit of fragility here, we need to make sure that we actually copypaste the exact right value here. But on the other hand, we even could make it intentionally something else, e.g. like this: #ifndef __MAC_10_15 // If the SDK doesn't define this constant, the SDK doesn't support this version anyway, and we won't end up selecting it, so just use a dummy value instead. #define __MAC_10_15 99999999 #endif What do you think, does any of that seem like it would make the code more manageable? // 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".