-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
Audio MSMF: added the ability to set sample per second #21145
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
Conversation
modules/videoio/src/cap_msmf.cpp
Outdated
if (value != 8000 && value != 11025 && value != 22050 && value != 44100) | ||
{ | ||
CV_LOG_ERROR(NULL, "VIDEOIO/MSMF: CAP_PROP_AUDIO_SAMPLES_PER_SECOND parameter value is invalid/unsupported: " << value); | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe MSMF framework should know better which data rate is supported. There is no limitation on OpenCV side (except may be >= 0 validation check). No need to duplicate MSMF logic in OpenCV code - no way to maintain this approach.
Need to ensure that MSMF should properly return "false" from .open()
with unsupported parameter value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add test with "invalid" / "non-supported" values.
cap.isOpened()
should return false
if .open()
parameters are not supported and/or not compatible.
modules/videoio/src/cap_msmf.cpp
Outdated
if (isOpen) | ||
{ | ||
if (audioStream != -1) | ||
checkAudioProperties(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkAudioProperties();
We should not ignore return value.
Function returns false on errors. Perhaps we should .close
capture object in that case.
When is configureAudioOutput()
called in this flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In open, first called configureOutput, then inside this function called configureAudioOutput. If I understood the question correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. This note is about setAudioProperties()
call above (line 1154)
modules/videoio/src/cap_msmf.cpp
Outdated
{ | ||
CV_LOG_ERROR(NULL, "VIDEOIO/MSMF: CAP_PROP_AUDIO_SAMPLES_PER_SECOND parameter value is invalid/unsupported: " << audioSamplesPerSecond | ||
<< ". Current value of CAP_PROP_AUDIO_SAMPLES_PER_SECOND: " << actualAudioSamplesPerSecond); | ||
captureAudioFormat.nSamplesPerSec = actualAudioSamplesPerSecond; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
captureAudioFormat.nSamplesPerSec = actualAudioSamplesPerSecond;
We should call close()
here because we can't satisfy requested parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you 👍
Audio MSMF: added the ability to set sample per second * Audio MSMF: added the ability to set sample per second * changed the valid sampling rate check * fixed docs * add test * fixed warning * fixed error * fixed error
I changed the CAP_PROP_AUDIO_SAMPLES_PER_SECOND property from (read-only) to (open, read). Acceptable values are 8.0 kHz, 11.025 kHz, 22.05 kHz, and 44.1 kHz. If the value is not set, the behavior is standard.
@alalek