GNOME Bugzilla – Bug 782695
alsa: Missing float PCM support
Last modified: 2017-05-20 17:03:14 UTC
Created attachment 351975 [details] [review] Patch for adding float pcm playback support Issue: alsasink not supporting FLOAT pcm playback. Root Cause: In gst-plugins-base-1.8.3/ext/alsa/gstalsa.c function "gst_alsa_get_pcm_format" switch case not added for FLOAT 32/64 pcm. By adding the switch I am able to play the float PCM files. Steps to verify: With the fix audioconvert is required. gst-launch filesrc location=<ogg file> ! decodebin ! audioconvert ! alsaink With fix audioconvert not required. gst-launch filesrc location=<ogg file> ! decodebin ! alsaink I have attached the fix as patch. I have verified the patch for FLOAT 32LE/64LE formats Thank you
Review of attachment 351975 [details] [review]: Thanks, Looks good in general. Can you rework the commit message ? Something like alsa: Add mapping for PCM F32/F64 formats . And in the long description, I'd mention that this enables float pcm formats for both the alsasrc and alsasink (your current message gives the impression this is only the sink). ::: ext/alsa/gstalsa.c @@ +131,3 @@ + case GST_AUDIO_FORMAT_F32BE: + return SND_PCM_FORMAT_FLOAT_BE; + /* 64 bit */ I think this comment is not needed.
Created attachment 352010 [details] [review] Add mapping for PCM F32 F64 formats Added proper commit message and fixed review comments
Comment on attachment 352010 [details] [review] Add mapping for PCM F32 F64 formats Looks good to me. Nicolas, you merge? Ideally you would've added a link to this bug report as the last line of the commit message, but when we merge this with "git bz", it will be automatically added anyway if missing.
Thanks for your patch.
Comment on attachment 352010 [details] [review] Add mapping for PCM F32 F64 formats commit f460d7d1844be815c918a47561ea1a5e8ef4795d - alsa: Missing float PCM support
Shouldn't this ticket be closed?