Skip to content
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

core: add mixChannels() method to Mat #746

Merged
merged 2 commits into from
Oct 21, 2020

Conversation

toffentoffen
Copy link
Contributor

Add MixChannels method for enhancement request #745

Comment on lines +1491 to +1519
cSrcMats := C.struct_Mats{
mats: (*C.Mat)(&cSrcArray[0]),
length: C.int(len(src)),
}

cDstArray := make([]C.Mat, len(dst))
for i, r := range dst {
cDstArray[i] = r.p
}
cDstMats := C.struct_Mats{
mats: (*C.Mat)(&cDstArray[0]),
length: C.int(len(dst)),
}

cFromToArray := make([]C.int, len(fromTo))
for i, ft := range fromTo {
cFromToArray[i] = C.int(ft)
}

cFromToIntVector := C.IntVector{
val: (*C.int)(&cFromToArray[0]),
length: C.int(len(fromTo)),
}

Copy link
Contributor Author

@toffentoffen toffentoffen Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added locally, accordingly all the defers close mats and close intvector:

	cSrcMats := C.struct_Mats{
		mats:   (*C.Mat)(&cSrcArray[0]),
		length: C.int(len(src)),
	}
	defer C.Mats_Close(cSrcMats)

	cDstArray := make([]C.Mat, len(dst))
	for i, r := range dst {
		cDstArray[i] = r.p
	}
	cDstMats := C.struct_Mats{
		mats:   (*C.Mat)(&cDstArray[0]),
		length: C.int(len(dst)),
	}
	defer C.Mats_Close(cDstMats)

	cFromToArray := make([]C.int, len(fromTo))
	for i, ft := range fromTo {
		cFromToArray[i] = C.int(ft)
	}

	cFromToIntVector := C.IntVector{
		val:    (*C.int)(&cFromToArray[0]),
		length: C.int(len(fromTo)),
	}
	defer C.IntVector_Close(cFromToIntVector)

But they are failing locally, for instance:

goroutine 0 [idle]:
runtime: unknown pc 0x7f1645a1c438

If I remove the defer the test passes, but the build fails because of TestMatProfile. Any clue what I am doing wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, at first glance you do not need any of the defers there. For example defer C.Mats_Close(cDstMats) would close the dst value, which is not what you would want right after returning.

I think perhaps the test itself it the issue. See comment below.

@@ -2015,6 +2015,44 @@ func TestMatMinMaxIdx(t *testing.T) {
}
}

func TestMixChannels(t *testing.T) {
bgra := NewMatWithSizeFromScalar(NewScalar(255, 0, 0, 255), 10, 10, MatTypeCV8UC4)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a defer bgra.Close() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

@@ -2015,6 +2015,44 @@ func TestMatMinMaxIdx(t *testing.T) {
}
}

func TestMixChannels(t *testing.T) {
bgra := NewMatWithSizeFromScalar(NewScalar(255, 0, 0, 255), 10, 10, MatTypeCV8UC4)
bgr := NewMatWithSize(bgra.Rows(), bgra.Cols(), MatTypeCV8UC3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a defer bgr.Close() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

func TestMixChannels(t *testing.T) {
bgra := NewMatWithSizeFromScalar(NewScalar(255, 0, 0, 255), 10, 10, MatTypeCV8UC4)
bgr := NewMatWithSize(bgra.Rows(), bgra.Cols(), MatTypeCV8UC3)
alpha := NewMatWithSize(bgra.Rows(), bgra.Cols(), MatTypeCV8UC1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a defer alpha.Close() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

@deadprogram
Copy link
Member

Also can you please rebase against the dev branch? That way the new CircleCI tests can run correctly. Thanks!

@toffentoffen
Copy link
Contributor Author

I don't know what I was thinking, after your explanation and after running the test locally with go test -run TestMixChannels -tags matprofile its so obvious 🤦
Thanks for your time and help.
Would you mind to add the label hacktoberfest-accepted to the PR? More info https://hacktoberfest.digitalocean.com/hacktoberfest-update

@deadprogram
Copy link
Member

Added hacktoberfest to repo itself.

@deadprogram
Copy link
Member

Now squash/merging. Thanks for another useful contribution @toffentoffen it is appreciated!

@deadprogram deadprogram merged commit eec3fce into hybridgroup:dev Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants