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

Refactor Vignette.Camera #235

Closed
wants to merge 9 commits into from
Closed

Conversation

Speykious
Copy link

@Speykious Speykious commented Oct 1, 2021

This PR fixes issue #234.

The previous solution that I've implemented is simply not adding a duplicate item in the FluentDropdown, and warning about it with a console write statement.
image
image

Now, the solution is indexing the friendly names so that all options pop up.
We're now faced with a "can't open camera by index" bug.

throw new ArgumentException($"The item {item} already exists in this {nameof(FluentDropdown<T>)}.");
{
/* throw new ArgumentException($"The item {item} already exists in this {nameof(FluentDropdown<T>)}."); */
System.Console.WriteLine($"\x1b[33mWarning: item {item} already exists in this {nameof(FluentDropdown<T>)}. Not adding it again.\x1b[0m");
Copy link

Choose a reason for hiding this comment

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

Use the Runtime log instead

Copy link
Author

Choose a reason for hiding this comment

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

Done :v

@Speykious
Copy link
Author

Now I've got another problem... It's not detecting my webcam and it says that it "can't open camera by index":

[performance] 2021-10-01 18:52:00 [verbose]: TextureAtlas initialised (1024x1024)
[runtime] 2021-10-01 18:52:01 [important]: Warning: item HD WebCam: HD WebCam
[runtime] 2021-10-01 18:52:01 [important]: already exists in this FluentDropdown. Not adding it again.
[ WARN:4] global /var/lib/buildbot/workers/worker/cv_ubuntu/build/opencv/modules/videoio/src/cap_v4l.cpp (890) open VIDEOIO(V4L2:/dev/video0): can't open camera by index
[performance] 2021-10-01 18:52:01 [verbose]: TextureAtlas size exceeded 1 time(s); generating new texture (1024x1024)

@Speykious
Copy link
Author

This might be relevant to the problem: opencv/opencv#19527

Maybe doing this is provides a better solution as it removes the need to
modify the fluent dropdown to be softer on duplicates.
@Speykious Speykious changed the title Webcam duplication fix Refactor Vignette.Camera Oct 2, 2021
@Speykious
Copy link
Author

Welp, time to rewrite this shit

@Speykious
Copy link
Author

Welp, time to not rewrite this shit

@Speykious
Copy link
Author

I'm gonna care about the reimplementation of Vignette.Camera for Encore 1. For now, I think I can focus on the "can't open camera by index" bug. There might be a problem with Camera instances as suggested there: https://stackoverflow.com/a/61817613

@sr229
Copy link
Member

sr229 commented Oct 3, 2021

I'm gonna care about the reimplementation of Vignette.Camera for Encore 1. For now, I think I can focus on the "can't open camera by index" bug. There might be a problem with Camera instances as suggested there: https://stackoverflow.com/a/61817613

I'll close this now since this will not be part of the Debut codebase, just remake what you did once we have the Stride code public :D

@sr229 sr229 closed this Oct 3, 2021
@Speykious
Copy link
Author

No no, wait! I just need to rename the pull request. I originally renamed it "Refactor Vignette.Camera" because I wanted to shift to implementing an FFmpeg version, but we still have to fix the webcam duplication and "can't open camera by index" bug. I can do it without a lot of rewrite in this PR.

@sr229
Copy link
Member

sr229 commented Oct 3, 2021

No no, wait! I just need to rename the pull request. I originally renamed it "Refactor Vignette.Camera" because I wanted to shift to implementing an FFmpeg version, but we still have to fix the webcam duplication and "can't open camera by index" bug. I can do it without a lot of rewrite in this PR.

Do it on another PR if that's the intent, we can't do it in this one since it's main context is replacing the backend and fixing the bugs.

@Speykious
Copy link
Author

That is its second context, which wasn't even really engaged in any significant modifications in the code fortunately.

@Speykious Speykious mentioned this pull request Oct 9, 2021
2 tasks
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.

None yet

3 participants