Audio GStreamer: added support .wav .flac audio formats#21264
Audio GStreamer: added support .wav .flac audio formats#21264alalek merged 4 commits intoopencv:4.xfrom
Conversation
| break; | ||
| } | ||
|
|
||
| return !dst.empty(); |
There was a problem hiding this comment.
empty result is valid in case when video stream is longer than audio.
There was a problem hiding this comment.
empty result is valid in case when video stream is longer than audio.
Within the framework of this functionality, such a case is not possible. I think this case should be considered separately by adding an additional condition when will audio + video support be implemented.
There was a problem hiding this comment.
Anyway this error criteria is not correct. And the error handling above in "default:".
We just forget to change that properly (our testdata doesn't handle such cases).
| /*! | ||
| * \brief CvCapture_GStreamer::retrieveFrame | ||
| * \return IplImage pointer. [Transfer Full] | ||
| * Retrieve the previously grabbed buffer, and wrap it in an IPLImage structure | ||
| */ |
There was a problem hiding this comment.
We can remove this outdated docstring
| } | ||
|
|
||
| #endif // BUILD_PLUGIN | ||
| #endif // BUILD_PLUGIN No newline at end of file |
There was a problem hiding this comment.
return EOL back.
Don't introduce unnecessary whitespace changes.
There was a problem hiding this comment.
return EOL back.
Don't introduce unnecessary whitespace changes.
Sorry, I do not quite understand what this is about
There was a problem hiding this comment.
Do you see changed code here? - yes
Is it indented and necessary change? - no
Then revert it.
| case CV_8S: | ||
| data = cv::Mat(1, audioFrame.rows, CV_8S); | ||
| for (int i = 0; i < audioFrame.rows; i++) | ||
| data.at<char>(0,i) = audioFrame.at<char>(i, index-audioBaseIndex); |
There was a problem hiding this comment.
(0,i)
Just (i).
.at(i) is agnostic to vector-rows or vector-cols.
| if (videoStream != -1) | ||
| return retrieveVideoFrame(index, dst); | ||
| else if (audioStream != -1) | ||
| return retrieveAudioFrame(index, dst); |
There was a problem hiding this comment.
This logic is not correct. Checks must be "index"-based.
What is the problem to write correct logic once instead of rewriting them again with adding video support?
| break; | ||
| } | ||
|
|
||
| return !dst.empty(); |
There was a problem hiding this comment.
Anyway this error criteria is not correct. And the error handling above in "default:".
We just forget to change that properly (our testdata doesn't handle such cases).
| Mat(map_info.size/bpf, nAudioChannels, CV_32F, map_info.data).copyTo(audioFrame); | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
missing the error message about unsupported format.
| data = cv::Mat(1, audioFrame.rows, CV_8S); | ||
| for (int i = 0; i < audioFrame.rows; i++) | ||
| data.at<char>(0,i) = audioFrame.at<char>(i, index-audioBaseIndex); | ||
| data.copyTo(dst); | ||
| break; |
There was a problem hiding this comment.
2 data copies are happened here.
We should avoid that to keep OpenCV effective library.
Use:
dst.create()data = dst.getMat()- fill
data
First 2 calls could be done outside of switch for successful path.
Audio GStreamer: added support .wav .flac audio formats * added support .wav, lossless compressed audio formats * fixed docs * fixes * videoio(gstreamer-audio): extra tests, improve error handling Co-authored-by: Alexander Alekhin <alexander.a.alekhin@gmail.com>
I added audio support wav and flac formats in Gstreamer backend