Skip to content

WIP: Implement 'ParallelImageCapture' for samd51 #4635

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

Merged
merged 14 commits into from
Apr 23, 2021
Merged

Conversation

jepler
Copy link

@jepler jepler commented Apr 19, 2021

This work-in-progress code, together with a WIP CircuitPython library, can successfully capture small (80x60) frames from an OV7670 camera on a Metro M4 GrandCentral.

TODO:

  • a name everyone can agree on
  • capture using DMA, rather than having interrupts disabled for 30ms

cap

jepler added 2 commits April 16, 2021 17:18
This is helpful when displaying frames from an OV7670 camera, which
uses the RGB565_SWAPPED format internally.
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks for the module rename. I like imagecapture. Just a couple other minor things.

I think you need per-param comments for the constructor too.

jepler added 5 commits April 20, 2021 10:21
24MHz is the nominal external clock to use with OV7670.  (My testing
was actually at 16MHz)
This allows ctrl-c to interrupt the capture, and ensures we handle
interrupts & background tasks.
@jepler jepler marked this pull request as ready for review April 20, 2021 18:53
@jepler
Copy link
Author

jepler commented Apr 20, 2021

@tannewt this is ready for a fresh review.

Main question I have right now is, I used "audio_dma" channels. As far as I can see this will work just fine (but didn't test): with 3 channels you can play stereo audio in the background and capture a framebuffer image in the foreground; we'd have to increase the channel count in the future when we enable this or audio capture to also happen in the background. That said, should the APIs audio_dma_allocate_channel() and audio_dma_free_channel be renamed? Be moved to different locations? Or can it be left as-is?

@jepler jepler requested a review from tannewt April 21, 2021 00:56
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

I think it would be good to rename the audio_dma stuff. I'm trying to remember what makes those special so we could rename them that way. Is it that they can be triggered by events?

jepler added 6 commits April 22, 2021 12:02
.. these do come in the right
.. this allows DIV_4 and DIV_2 resolutions to work without being
jumbled.
These are now used in the (video) parallel capture device as well.
@jepler
Copy link
Author

jepler commented Apr 23, 2021

@tannewt this is ready for a fresh review assuming the CI passes. Let me know if I need to merge main or rebase onto it.

tannewt
tannewt previously approved these changes Apr 23, 2021
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Code looks good. Thank you! You'll need to fix the CI though.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 80e8b4a into adafruit:main Apr 23, 2021
@jepler
Copy link
Author

jepler commented Apr 24, 2021

@ladyada
Test program for ov7670 + gcm4 + 1.8" tft shield: https://gist.github.com/645f7d052b63103111b011e507df38a1
Test program for ov7670 + rpi pico + 2.0" ST7789 breakout: https://gist.github.com/f357464e194a4ba85bbcd43378b1e189

I think the 2nd one needs the fix for the DIV2 resolution mode, adafruit/Adafruit_CircuitPython_OV7670#4

@jepler jepler deleted the pcc branch November 3, 2021 21:10
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