Skip to content

AVFoundation support for macOS #7159

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 3 commits into from
Sep 16, 2016
Merged

AVFoundation support for macOS #7159

merged 3 commits into from
Sep 16, 2016

Conversation

rgov
Copy link

@rgov rgov commented Aug 23, 2016

resolves #4930
resolves #6913

I did not write these patches, but I've rebased them onto master and tested that they work. I squashed commits related to whitespace issues.

The work was originally done by @mself and @yoffy in #6678, which was closed and re-opened as #6802 by @vpisarev, who also opened #6826 with a subset of the original patches. That one now does not merge cleanly. Then GitHub seemed to lose some of the history (maybe someone deleted their fork?) and it was a little painful to reconstruct the patches by copying and pasting from the web UI.

yoffy and others added 3 commits August 23, 2016 10:51
Support setting CAP_PROP_MODE to capture grayscale or YUV frames much
faster from CV_CAP_AVFOUNDATION_MAC.
CvVideoWriter_AVFoundation_Mac had a serious buffer release bug.

Also made writeFrame() block until isReadyForMoreMediaData rather than
return an error.
@mself
Copy link
Contributor

mself commented Aug 23, 2016

@rgov, thank you for cleaning these patches up. I will review and re-test to confirm everything.

@rgov
Copy link
Author

rgov commented Aug 25, 2016

Ping.

@mself
Copy link
Contributor

mself commented Aug 25, 2016

OK, I reviewed and all of the changes appear to be in correctly. LGTM!

@kushalj1997
Copy link

Can we get this fix into the main fork? So that I can begin using OpenCV on macOS Sierra again? Thanks!

@mself
Copy link
Contributor

mself commented Aug 27, 2016

What is the expected behavior of CV_CAP_PROP_POS_MSEC and CV_CAP_PROP_POS_FRAMES? Currently, CV_CAP_PROP_POS_MSEC returns the timestamp of the last frame that was read. But CV_CAP_PROP_POS_FRAMES returns the frame number of the next frame that will be read.

For example, if you open a CvCaptureFile and read these properties right away you get frame = 0 and msec = 0.0. After reading one frame, you get frame = 1 and still get msec = 0.0.

There is no way to reliably know the timestamp of the next frame, so it makes sense to return the actual timestamp of the previous frame. Should we make the frame numbers behave the same way? Should callers use getProperty() to get info on the last frame they read?

If so, should CV_CAP_PROP_POS_FRAMES return -1 if you call it before reading any frames? This seems like it could be a breaking change. Or should it just return 0 in both cases (like CV_CAP_PROP_POS_MSEC currently does)?

(I'm assuming here that frame numbers are zero-based on OpenCV, but I don't see that documented clearly anywhere.)

Currently my app uses a hacky -1 to adjust the frame number that I get back from CV_CAP_PROP_POS_FRAMES.

@rgov
Copy link
Author

rgov commented Sep 1, 2016

@mself Are these (perfectly valid) open questions blocking acceptance of this pull request? If not, how do we get this merged in first?

@mself
Copy link
Contributor

mself commented Sep 2, 2016

@rgov Sorry, no, I don't think my question should in any way hold up merging this PR. I'm using this code in my app today and it works fine. It seems like there is a lot of demand for getting this merged.

@pr0methaz1ne
Copy link

So is this merged yet? Thanks

@PkLab
Copy link
Contributor

PkLab commented Sep 9, 2016

Is there any doc, sample, snippet or tutorial for AVFoundation for macOS ?

@mself
Copy link
Contributor

mself commented Sep 9, 2016

Here's a simple example:

VideoCapture stream;
stream.open(filename, CV_CAP_AVFOUNDATION_MAC);  // Use AVF to read the file
stream.set(CAP_PROP_MODE, CV_CAP_MODE_GRAY);  // For grayscale mode (default is RGB)
bool result = stream.read(mat);  // Read a frame

@PkLab
Copy link
Contributor

PkLab commented Sep 9, 2016

it would be nice to see anything in the official docs ;) maybe here ?

@yoffy
Copy link
Contributor

yoffy commented Sep 10, 2016

I think timestamp of getProperty will be next frame. Because, setProperty means to set next frame.

But, @mself says:

There is no way to reliably know the timestamp of the next frame, so it makes sense to return the actual timestamp of the previous frame.

This is true.
Is there any good ideas to solve this problem and avoiding additional next frame reading?

@briantopping
Copy link

I really regret upgrading my machines to Sierra as a result of this issue. Is there any way to get a working branch (even if sub-optimal) and work on long-term solutions with additional PRs?

@mself
Copy link
Contributor

mself commented Sep 11, 2016

This branch works fine. I am using it on Sierra for all of my projects. The discussion about how getProperty() should work is unrelated to the AVF support (and probably applies to other videoio drivers besides the AVF one).

I also strongly support merging this PR.

@briantopping
Copy link

briantopping commented Sep 11, 2016

@mself: Strange, NM. Compiled the branch on CLion on the same box and it compiles fine. Thanks for your effort on the fork and trying to get it merged!

@heinemml
Copy link

With XCode 8 released QTKit support is now also gone on El Capitan. So AVF support is now really important.

@barronlroth
Copy link

I don't mean to beat a dead horse, but install is currently impossible on Sierra due to QTKit removal. Any update would be very much appreciated.

https://github.com/Homebrew/homebrew-science/issues/4303

@mshabunin
Copy link
Contributor

Please check whether #7266 (based on this PR) resolves the issue.

@opencv-pushbot opencv-pushbot merged commit 0882936 into opencv:master Sep 16, 2016
@marcusguttenplan
Copy link

Merged, great job! How long until this appears downstream in brew?

@briantopping
Copy link

+1

@marcusguttenplan: Have you ever sent a PR to brew? I did one some months back, it's super easy.

@marcusguttenplan
Copy link

@briantopping I have not! I assume it's just a matter of updating the branch and adding a new checksum for Sierra in homebrew-science? I don't want to break anything though.

@briantopping
Copy link

That’s correct. You what to expect from the software and there are hundreds of packages being upgraded all the time. If it works for you, it works better than what is there now. :-)

Go for it!

@marcusguttenplan
Copy link

Depending on how often OpenCV pushes changes from the master branch to the stable 2.4 branch, it should start appearing in homebrew tomorrow. I'm weary of changing the branch in the brew formula to the master in case of future breakage, and there's already a checksum for Sierra. Looks like I've just got to wait giddily!

@alalek
Copy link
Member

alalek commented Sep 16, 2016

This patch should be available now in the --HEAD build of homebrew-science/opencv3

@jordimares
Copy link

Thanks! Does anyone know if this fix is also available for OpenCV 2.4?

@alalek
Copy link
Member

alalek commented Sep 19, 2016

@jordimares There is backport of this patch into 2.4 branch: #7314. Could you take a look?

@jordimares
Copy link

@alalek Thanks for this. Any idea when can it be merged so I can install it in my macOS Sierra?

@tmenguy
Copy link

tmenguy commented Sep 26, 2016

same here for macports ...

@shish
Copy link

shish commented Oct 19, 2016

For any other macports users who come across this bug report, this is how I got installation to work with video disabled (which is good enough for me):

  • sudo vim /opt/local/var/macports//sources/rsync.macports.org/release/tarballs/ports/graphics/opencv/Portfile
  • find the list of build options, eg search for DBZIP2_LIBRARIES
  • Add a backslash on that line
  • Add on the next line: -DBUILD_opencv_videoio=OFF
  • sudo port install opencv +python35 now works

@PifPof73
Copy link

That worked for me. Thanx, you are my weekend-hero!

csjune pushed a commit to crema/nuvo-image that referenced this pull request Nov 30, 2016
### 문제점
- 애플이 sierra에서 qtkit 없애버림
- opencv/opencv#7159 에서 수정됐지만 아직 정식으로 배포되진 않았음

### 수정 사항
- mac os x일 때는 최신 버전 opencv를 빌드하도록 수정함
iMichka added a commit to iMichka/homebrew-science that referenced this pull request Jun 30, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Since opencv/opencv#7159,
opencv and opencv3 use AVFoundation instead of quicktime.
The same change as in this commit was done for the opencv
formula, so this brings both opencv and opencv3 in line.

The AVFoundation is ON by default on mac, so video capabilities
are still guaranteed.
iMichka added a commit to iMichka/homebrew-science that referenced this pull request Jun 30, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Since opencv/opencv#7159,
opencv and opencv3 use AVFoundation instead of quicktime.
The same change as in this commit was done for the opencv
formula, so this brings both opencv and opencv3 in line.

The AVFoundation is ON by default on mac, so video capabilities
are still guaranteed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet