New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#292: Support ArUco #343

Merged
merged 4 commits into from May 5, 2017

Conversation

Projects
None yet
2 participants
@takuya-takeuchi
Copy link
Contributor

takuya-takeuchi commented May 3, 2017

Add elementary functions for Aruco class, DetectorParameters class , Dictionary class and PredefinedDictionaryName enum and test class.

@takuya-takeuchi

This comment has been minimized.

Copy link
Contributor

takuya-takeuchi commented May 3, 2017

AppVayorのテストはPassしているように見えるのですが、どうしてエラーなのかがわかりません。
必要なら修正しますので、詳細を教えていただけないでしょうか?

@shimat shimat added the enhancement label May 5, 2017

@shimat

This comment has been minimized.

Copy link
Owner

shimat commented May 5, 2017

なんとここまでして頂けるとは。arucoは面倒そうで尻込みしていました。ありがとうございます!

Passしているようでappveyorが落ちている件は結構難しそうでまだ分からずですが、これまでの経験で言いますと、何かしらメモリの破壊だとかdelete済みのポインタを使っただとかでクラッシュすると、こうなったことがあります。ローカルでテストを実行しても大体は再現します。もしかするとというのが思いついた箇所はコメントで示します。

throw new ArgumentNullException(nameof(image));

IntPtr cornersPtr, idsPtr, rejectedImgPointsPtr;
NativeMethods.aruco_detectMarkers(image.CvPtr, dictionary.CvPtr, out cornersPtr, out idsPtr, parameters.CvPtr, out rejectedImgPointsPtr);

This comment has been minimized.

@shimat

shimat May 5, 2017

Owner

最適化がかかっていると、ここでdictionary.CvPtrを渡した後はもうそのIntPtrだけ残っていればよくてDictionary自体はいらないだろう、とGCが発動して解放されてしまうことが稀にあります。するとdelete済みのCvPtrが使われることになりクラッシュします。
DetectMarkers(new Mat(), new Dictionary()) のような呼ばれ方をしてしまうと可能性UPです。

非常にめんどくさくて既存実装でも抜けが多いですが、ひたすら GC.KeepAlive(dictionary); のような記述をメソッドの最後に置いておくのが必要です。

@shimat

shimat approved these changes May 5, 2017

@takuya-takeuchi

This comment has been minimized.

Copy link
Contributor

takuya-takeuchi commented May 5, 2017

Approvedしてくださったということは、もうこっちからは修正しなくて良いと言うことでしょうか?

@shimat

This comment has been minimized.

Copy link
Owner

shimat commented May 5, 2017

はい。マージしてからおいおい修正でも良いかなと...。重ねて、ありがとうございます。

@shimat shimat merged commit bfaf35f into shimat:master May 5, 2017

1 check failed

continuous-integration/appveyor/pr AppVeyor build failed
Details

@takuya-takeuchi takuya-takeuchi deleted the takuya-takeuchi:feature/aruco branch May 5, 2017

@shimat

This comment has been minimized.

Copy link
Owner

shimat commented May 11, 2017

@takuya-takeuchi
cv::Ptr<T> を引数に取るところに T* を渡すと、暗黙の型変換で一見うまくいきます。しかし、その場で作られたcv::Ptrオブジェクトは参照カウント0になるため関数が終わると即座に解放され、渡したT*がdeleteされます。
そのあとでOpenCvSharpによってもう一度deleteされてしまいます。これが良くなかったようです。
2c80bdb#diff-d7f40f3453aef52908cef404b1a9311bR48

うまい解決策はあまり思いついていなくて、最初からcv::Ptr<T>で引数を渡すように変えてこれまで回避してきています。これでうまくいったようです。
https://ci.appveyor.com/project/shimat/opencvsharp/build/3.2.0.20170511-beta-79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment