[CoreVideo] Add support for Xcode 13 beta3. #12257

mandel-macaque · 2021-07-27T17:30:56Z

No description provided.

stephen-hawley · 2021-07-27T19:12:46Z

src/CoreVideo/CVDisplayLink.cs

+ public bool TryTranslateTime (CVTimeStamp inTime, out CVTimeStamp? outTime)
+ {
+ outTime = null;
+ if (CVDisplayLinkTranslateTime (this.Handle, inTime, out var translated) == 0) {


The documentation says this:

outTime A pointer to a CVTimeStamp structure into which the target time is written. Before calling, you must set the version field (currently 0) to indicate which version of the structure you want. You should also set the flags field to specify which representations to translate to.

You're passing in null, which is not what the API calls for. It looks from the documentation that this should be a ref parameter and not an out parameter, and outTime.Version should be 0.

Nice catch, I'll update it, thx!

spouliot · 2021-07-27T19:56:39Z

src/CoreVideo/CVBuffer.cs

 return Runtime.GetINativeObject<T> (CVBufferGetAttachment (handle, key.Handle, out attachmentMode), false);
 }
 #else
 public NSObject GetAttachment (NSString key, out CVAttachmentMode attachmentMode)
 {
 if (key == null)
 throw new ArgumentNullException ("key");
- return Runtime.GetNSObject (CVBufferGetAttachment (handle, key.Handle, out attachmentMode));
+ if (PlatformHelper.CheckSystemVersion (12, 0))
+ return Runtime.GetNSObject (CVBufferCopyAttachment (handle, key.Handle, out attachmentMode));


missing argument for ownership

The API without the <T> does not have the argument: https://docs.microsoft.com/en-us/dotnet/api/objcruntime.runtime.getnsobject?view=xamarin-ios-sdk-12#ObjCRuntime_Runtime_GetNSObject_System_IntPtr_ Which is the one being used, should I switch to the version?

yup, the generic version is newer/better
the alternative would be to explicitly call memory managed methods... which is less desirable

This still needs to be fixed

spouliot · 2021-07-27T20:02:58Z

src/CoreVideo/CVBuffer.cs

+#elif MONOMAC
+ if (PlatformHelper.CheckSystemVersion (12, 0))
+#endif
+ return (NSDictionary) Runtime.GetNSObject (CVBufferCopyAttachments (handle, attachmentMode));


same, you're not specifying the ownership (using the default)

src/CoreVideo/CVDisplayLink.cs

spouliot · 2021-07-27T20:06:51Z

src/CoreVideo/CVDisplayLink.cs

+ IntPtr handle = IntPtr.Zero;
+ unsafe {
+ fixed (uint *displaysHandle = displayIds) {
+ result = CVDisplayLinkCreateWithCGDisplays (displaysHandle, displayIds.Length, out handle);


NullReferenceException if displayIds is null - which is not validated

I wonder why #nullable enable did not report an error...

src/CoreVideo/CVDisplayLink.cs

spouliot · 2021-07-27T20:10:38Z

src/CoreVideo/CVDisplayLink.cs

+ static extern int CVDisplayLinkTranslateTime (IntPtr displayLink, CVTimeStamp inTime, ref CVTimeStamp outTime);
+
+ [Mac (12,0), NoiOS, NoTV]
+ public bool TryTranslateTime (CVTimeStamp inTime, out CVTimeStamp? outTime)


Use [NotNullWhen (true)] instead of ?
https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.notnullwhenattribute?view=net-5.0

vs-mobiletools-engineering-service2 · 2021-07-28T00:54:36Z

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

iOS vsdrops gist
macOS vsdrops gist
macCat vsdrops gist
macCatiOS gist
tvOS vsdrops gist
watchOS vsdrops gist

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

3 tests failed, 86 tests passed.

Failed tests

introspection/Mac Modern/Debug: Failed (Test run failed.
Tests run: 37 Passed: 35 Inconclusive: 0 Failed: 1 Ignored: 1)
introspection/Mac Catalyst/Debug: Failed (Tests run: 44 Passed: 38 Inconclusive: 0 Failed: 1 Ignored: 5)
monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2629 Passed: 2488 Inconclusive: 35 Failed: 1 Ignored: 140)

Pipeline on Agent XAMBOT-1096.BigSur'
Merge c33bb04 into f611cd8

rolfbjarne

Also manual bindings need manual tests 😄

rolfbjarne · 2021-07-28T07:53:12Z

src/CoreVideo/CVMetalTextureCache.cs

@@ -21,7 +21,7 @@

 namespace CoreVideo {

- [iOS (8,0)]
+ [iOS (8,0), Mac (12,0)]


Mac Catalyst?

rolfbjarne · 2021-07-28T07:53:21Z

src/CoreVideo/CVMetalTexture.cs

@@ -22,7 +22,7 @@

 namespace CoreVideo {

- [iOS (8,0)]
+ [iOS (8,0), Mac (12,0)]


Mac Catalyst?

rolfbjarne · 2021-07-28T07:54:39Z

src/CoreVideo/CVBuffer.cs

 [DllImport (Constants.CoreVideoLibrary)]
 extern static /* CFTypeRef */ IntPtr CVBufferGetAttachment (/* CVBufferRef */ IntPtr buffer, /* CFStringRef */ IntPtr key, out CVAttachmentMode attachmentMode);

+ // The new method is the same as the old one but changing the ownership from Get to Copy, so we will use the new version if possible since the
+ // older method has been deprecatd.
+ [Watch (8,0), TV (15,0), Mac (12,0), iOS (15,0)]


Mac Catalyst?

rolfbjarne · 2021-07-28T08:41:06Z

src/CoreVideo/CVBuffer.cs

 [DllImport (Constants.CoreVideoLibrary)]
 extern static /* CFDictionaryRef */ IntPtr CVBufferGetAttachments (/* CVBufferRef */ IntPtr buffer, CVAttachmentMode attachmentMode);

+ [Watch (8,0), TV (15,0), Mac (12,0), iOS (15,0)]


Mac Catalyst?

src/CoreVideo/CVBuffer.cs

src/CoreVideo/CVDisplayLink.cs

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>

vs-mobiletools-engineering-service2 · 2021-07-28T15:51:58Z

🔥 Tests failed catastrophically on Build (no summary found). 🔥

Result file $(TEST_SUMMARY_PATH) not found.

Pipeline on Agent
Merge 21b7238 into 3336910

This reverts commit dcbbdf3.

src/CoreVideo/CVDisplayLink.cs

vs-mobiletools-engineering-service2 · 2021-07-28T23:56:22Z

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

iOS vsdrops gist
macOS vsdrops gist
macCat vsdrops gist
macCatiOS gist
tvOS vsdrops gist
watchOS vsdrops gist

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

15 tests failed, 74 tests passed.

Failed tests

introspection/Mac Modern/Debug: Failed (Test run failed.
Tests run: 37 Passed: 35 Inconclusive: 0 Failed: 1 Ignored: 1)
introspection/Mac Catalyst/Debug: Failed (Tests run: 44 Passed: 38 Inconclusive: 0 Failed: 1 Ignored: 5)
monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2629 Passed: 2489 Inconclusive: 35 Failed: 2 Ignored: 138)
monotouch-test/iOS Unified 64-bits - simulator/Debug: Failed
monotouch-test/iOS Unified 64-bits - simulator/Debug (LinkSdk): Failed
monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar): Failed
monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations): Failed
monotouch-test/tvOS - simulator/Debug: Failed
monotouch-test/tvOS - simulator/Debug (LinkSdk): Failed
monotouch-test/tvOS - simulator/Debug (static registrar): Failed
monotouch-test/tvOS - simulator/Release (all optimizations): Failed
monotouch-test/watchOS 32-bits - simulator/Debug: Failed
monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): Failed
monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): Failed
monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): Failed

Pipeline on Agent XAMBOT-1104.BigSur'
Merge ba74e6f into 3336910

vs-mobiletools-engineering-service2 · 2021-08-18T22:03:10Z

🔥 Tests failed catastrophically on Build (no summary found). 🔥

Result file $(TEST_SUMMARY_PATH) not found.

Pipeline on Agent
Merge 43a87fc into 243cc40

…amarin-macios into corevideo-xcode13-beta3

tj-devel709 · 2021-09-01T16:51:13Z

tests/monotouch-test/CoreVideo/CVDisplayLinkTest.cs

+ }
+
+ [Test]
+ public void DefaultconstructorTest () => Assert.DoesNotThrow (() => {


nit: DefaultconstructorTest -> DefaultConstructorTest

spouliot · 2021-09-01T16:52:07Z

src/CoreVideo/CVBuffer.cs

- return (NSDictionary) Runtime.GetNSObject (CVBufferCopyAttachments (handle, attachmentMode));
- return (NSDictionary) Runtime.GetNSObject (CVBufferGetAttachments (handle, attachmentMode));
+ return Runtime.GetINativeObject<NSDictionary> (CVBufferCopyAttachments (handle, attachmentMode), true);
+ return Runtime.GetINativeObject<NSDictionary> (CVBufferGetAttachments (handle, attachmentMode), false);


Runtime.GetNSObject<NSDictionary> would be better since you know the return type (and GetINativeObject is more complex)

spouliot · 2021-09-01T16:56:54Z

src/CoreVideo/CVBuffer.cs

@@ -150,9 +151,9 @@ public NSObject GetAttachment (NSString key, out CVAttachmentMode attachmentMode
 if (key == null)
 throw new ArgumentNullException ("key");
 if (PlatformHelper.CheckSystemVersion (12, 0))
- return Runtime.GetNSObject (CVBufferCopyAttachment (handle, key.Handle, out attachmentMode));
+ return Runtime.GetINativeObject<NSObject> (CVBufferCopyAttachment (handle, key.Handle, out attachmentMode), true);


Runtime.GetNSObject<NSObject> (ptr, true)

since GetINativeObject logic is more complex and you already know the return type.

spouliot · 2021-09-01T16:57:17Z

src/CoreVideo/CVBuffer.cs

 else
- return Runtime.GetNSObject (CVBufferGetAttachment (handle, key.Handle, out attachmentMode));
+ return Runtime.GetINativeObject<NSObject> (CVBufferGetAttachment (handle, key.Handle, out attachmentMode), false);


spouliot · 2021-09-01T16:59:38Z

tests/monotouch-test/CoreVideo/CVDisplayLinkTest.cs

+ {
+ // we need to have access to the media display to be able to perform
+ // the displaylink tests
+ mainDisplayId = CGDisplay.MainDisplayID;


mainDisplayId does not seem to be used (except in TearDown)
are you missing an Ignore in case no valid is was returned ?

I can remove SetUp and TearDown.

vs-mobiletools-engineering-service2 · 2021-09-01T20:40:03Z

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

iOS vsdrops gist
macOS vsdrops gist
macCat vsdrops gist
macCatiOS gist
tvOS vsdrops gist
watchOS vsdrops gist

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 107 tests passed.

Failed tests

introspection/Mac Catalyst/Debug: Failed (Tests run: 44 Passed: 39 Inconclusive: 0 Failed: 1 Ignored: 4)

Pipeline on Agent XAMBOT-1096.BigSur'
Merge 94d8143 into 5b896c7

vs-mobiletools-engineering-service2 · 2021-09-02T20:52:08Z

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

iOS vsdrops gist
macOS vsdrops gist
macCat vsdrops gist
macCatiOS gist
tvOS vsdrops gist
watchOS vsdrops gist

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

2 tests failed, 106 tests passed.

Failed tests

monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2644 Passed: 2490 Inconclusive: 35 Failed: 1 Ignored: 153)
introspection/Mac Catalyst/Debug: Failed (Tests run: 44 Passed: 39 Inconclusive: 0 Failed: 1 Ignored: 4)

Pipeline on Agent XAMBOT-1094.BigSur'
Merge 050f6f0 into f9c0589

vs-mobiletools-engineering-service2 · 2021-09-08T21:22:40Z

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

iOS vsdrops gist
macOS vsdrops gist
macCat vsdrops gist
macCatiOS gist
tvOS vsdrops gist
watchOS vsdrops gist

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

2 tests failed, 100 tests passed.

Failed tests

link sdk/tvOS - simulator/Debug: Failed
link sdk/tvOS - simulator/Release: Failed

Pipeline on Agent XAMBOT-1103.BigSur'
Merge e522eb4 into c2fda6e

src/CoreVideo/CVBuffer.cs

spouliot · 2021-09-08T21:36:19Z

src/CoreVideo/CVBuffer.cs

+ public bool HasAttachment (NSString key)
+ {
+ if (key == null)
+ throw new ArgumentNullException (nameof (key));


minor: nicer (smaller code and no potential == overload to mess things up)

if (key is null) ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (key));

src/CoreVideo/CVBuffer.cs

vs-mobiletools-engineering-service2 · 2021-09-10T02:26:01Z

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

iOS vsdrops gist
macOS vsdrops gist
macCat vsdrops gist
macCatiOS gist
tvOS vsdrops gist
watchOS vsdrops gist

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

5 tests failed, 97 tests passed.

Failed tests

fsharp/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut (Execution timed out after 1200 seconds.
No test log file was produced)
interdependent-binding-projects/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut (Execution timed out after 1200 seconds.
No test log file was produced)
Xtro/Mac: Failed (Test run failed.)
link sdk/tvOS - simulator/Debug: Failed
link sdk/tvOS - simulator/Release: Failed

Pipeline on Agent XAMBOT-1098.BigSur'
Merge d44a025 into 55e8c60

vs-mobiletools-engineering-service2 · 2021-09-10T21:56:13Z

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

iOS vsdrops gist
macOS vsdrops gist
macCat vsdrops gist
macCatiOS gist
tvOS vsdrops gist
watchOS vsdrops gist

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

3 tests failed, 99 tests passed.

Failed tests

dont link/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut (Execution timed out after 1200 seconds.
No test log file was produced)
link sdk/tvOS - simulator/Debug: Failed
link sdk/tvOS - simulator/Release: Failed

Pipeline on Agent XAMBOT-1104.BigSur'
Merge 3949601 into 42dd6ce

mandel-macaque · 2021-09-13T15:08:22Z

Known issues: #12640

mandel-macaque added 6 commits July 22, 2021 16:19

[HealthKit] Add support for xcode13 beta3.

da24cab

Address reviews.

7d79213

Merge branch 'main' into healthkit-xcode13-beta3

7b35179

Add missing selector for WatchOS 8.

915f8e6

[CoreVideo] Add support for Xcode13 beta 3.

2b32222

[CoreVideo] Add support for Xcode13 beta 3

03026b5

mandel-macaque added the note-highlight Worth calling out specifically in release notes label Jul 27, 2021

mandel-macaque requested review from dalexsoto, rolfbjarne and spouliot as code owners July 27, 2021 17:30

stephen-hawley requested changes Jul 27, 2021

View reviewed changes

mandel-macaque added 2 commits July 27, 2021 15:29

Remove method from watchos.

dcbbdf3

Address reviews.

ecb3a76

stephen-hawley approved these changes Jul 27, 2021

View reviewed changes

spouliot suggested changes Jul 27, 2021

View reviewed changes

Address reviews.

b1e0ec9

spouliot approved these changes Jul 27, 2021

View reviewed changes

Add overload to return the error.

c33bb04

rolfbjarne requested changes Jul 28, 2021

View reviewed changes

Apply suggestions from code review

21b7238

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>

Revert "Remove method from watchos."

43657fc

This reverts commit dcbbdf3.

tj-devel709 reviewed Jul 28, 2021

View reviewed changes

src/CoreVideo/CVDisplayLink.cs Outdated Show resolved Hide resolved

tj-devel709 approved these changes Jul 28, 2021

View reviewed changes

mandel-macaque added 2 commits July 28, 2021 12:38

Fix intro on sim.

127806e

Address reviews and fix compilation.

ba74e6f

mandel-macaque added 2 commits July 28, 2021 20:12

Merge branch 'main' into corevideo-xcode13-beta3

76d8cc2

Fix comments from review, update for beta 4.

2aba632

Fix mac intro.

43a87fc

mandel-macaque added 3 commits August 31, 2021 12:28

Merge branch 'corevideo-xcode13-beta3' of github.com:mandel-macaque/x…

be1614e

…amarin-macios into corevideo-xcode13-beta3

Merge branch 'main' into corevideo-xcode13-beta3

1da2f02

Address reviews and add tests for manual code.

94d8143

chamons approved these changes Sep 1, 2021

View reviewed changes

tj-devel709 reviewed Sep 1, 2021

View reviewed changes

spouliot suggested changes Sep 1, 2021

View reviewed changes

mandel-macaque added 3 commits September 1, 2021 17:34

Fix api and testt.

b141a30

Merge branch 'main' into corevideo-xcode13-beta3

050f6f0

Merge branch 'main' into corevideo-xcode13-beta3

5c17e7d

Fix issue with the catalyst intro tests.

e522eb4

mandel-macaque commented Sep 8, 2021

View reviewed changes

src/CoreVideo/CVBuffer.cs Outdated Show resolved Hide resolved

mandel-macaque commented Sep 8, 2021

View reviewed changes

src/CoreVideo/CVBuffer.cs Outdated Show resolved Hide resolved

spouliot reviewed Sep 8, 2021

View reviewed changes

mandel-macaque commented Sep 8, 2021

View reviewed changes

src/CoreVideo/CVBuffer.cs Outdated Show resolved Hide resolved

mandel-macaque commented Sep 8, 2021

View reviewed changes

src/CoreVideo/CVBuffer.cs Outdated Show resolved Hide resolved

Apply suggestions from code review

ac7a987

spouliot approved these changes Sep 8, 2021

View reviewed changes

rolfbjarne approved these changes Sep 9, 2021

View reviewed changes

Merge branch 'main' into corevideo-xcode13-beta3

d44a025

Merge branch 'main' into corevideo-xcode13-beta3

3949601

mandel-macaque merged commit f1611df into xamarin:main Sep 13, 2021

mandel-macaque deleted the corevideo-xcode13-beta3 branch September 13, 2021 15:09

[CoreVideo] Add support for Xcode 13 beta3. #12257

[CoreVideo] Add support for Xcode 13 beta3. #12257

Conversation

mandel-macaque commented Jul 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mandel-macaque Jul 27, 2021 • edited by spouliot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vs-mobiletools-engineering-service2 commented Jul 28, 2021

❌ [PR Build] Tests failed on Build ❌

API diff

API & Generator diff

GitHub pages

Test results

Failed tests

rolfbjarne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vs-mobiletools-engineering-service2 commented Jul 28, 2021

🔥 Tests failed catastrophically on Build (no summary found). 🔥

vs-mobiletools-engineering-service2 commented Jul 28, 2021

❌ [PR Build] Tests failed on Build ❌

API diff

API & Generator diff

GitHub pages

Test results

Failed tests

vs-mobiletools-engineering-service2 commented Aug 18, 2021

🔥 Tests failed catastrophically on Build (no summary found). 🔥

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vs-mobiletools-engineering-service2 commented Sep 1, 2021

❌ [PR Build] Tests failed on Build ❌

API diff

API & Generator diff

GitHub pages

Test results

Failed tests

vs-mobiletools-engineering-service2 commented Sep 2, 2021

❌ [PR Build] Tests failed on Build ❌

API diff

API & Generator diff

GitHub pages

Test results

Failed tests

vs-mobiletools-engineering-service2 commented Sep 8, 2021

❌ [PR Build] Tests failed on Build ❌

API diff

API & Generator diff

GitHub pages

Test results

Failed tests

Choose a reason for hiding this comment

vs-mobiletools-engineering-service2 commented Sep 10, 2021

❌ [PR Build] Tests failed on Build ❌

API diff

API & Generator diff

GitHub pages

Test results

Failed tests

vs-mobiletools-engineering-service2 commented Sep 10, 2021

❌ [PR Build] Tests failed on Build ❌

API diff

API & Generator diff

mandel-macaque Jul 27, 2021 •

edited by spouliot

Loading