search

ros-drivers / audio_common

Common code for working with audio in ROS
BSD 3-Clause "New" or "Revised" License
86 stars 151 forks source link

[audio_common_msgs] add AudioDataStamped.msg #202

Closed knorth55 closed 1 year ago

knorth55 commented 1 year ago

related to #201 I add AudioDataStamped msg, which has Header.

708yamaguchi commented 1 year ago

Should AudioInfo fileld be added to AudioDataStamped msg? https://github.com/ros-drivers/audio_common/blob/master/audio_common_msgs/msg/AudioInfo.msg

knorth55 commented 1 year ago

Hmm, we need discussion about it. IMO, AudioInfo is not required to be published in high frequency as CameraInfo. But, I understand all information should be in one message, which is easier for users.

v4hn commented 1 year ago

I agree with @knorth55 . CameraInfo can basically adapt parameters to changes in focal length or different modes of the camera. For AudioInfo the main use cases would be

  1. changing number of channels. The only use-case where I might see that is for sound source separation with a previously unknown number of sources. I'm not convinced this use-case is worth adding the overhead everywhere.
  2. variable bit-rate (VBR), but that's already represented in MP3 if used and bitrate would be redundant
  3. image_transport-style transparent transport switching (pcm-mp3-ogg). I believe if someone wants that, they would implement audio_transport instead.

I very much prefer to only look at AudioInfo once on startup and verify/setup the subscriber accordingly.

708yamaguchi commented 1 year ago

@v4hn Thank you for your insights. I agree with your idea.

v4hn commented 1 year ago

Great, thanks. Now somebody just has to implement a proper time reading from libgstreamer in audio_capture. Until then even the seq number in the header can already help detect dropped packages due to ROS (however not due to the gstreamer pipeline!). :) Assuming you want to publish audio_stamped in addition to audio from there now.

knorth55 commented 1 year ago

Now somebody just has to implement a proper time reading from libgstreamer in audio_capture.

I think the first implemtation is like you did in master...v4hn:audio_common:master-audio-stamped#diff-9a3a6c75b5. I need to find the better way, but I don't have enough time now.

Assuming you want to publish audio_stamped in addition to audio from there now.

Yes :)