GNOME Bugzilla – Bug 759484
directsoundsrc: add device property as it is done in directsoundsink
Last modified: 2015-12-18 11:40:33 UTC
Created attachment 317410 [details] [review] add guid device property as it is done in directsoundsink add guid device property as it is done in directsoundsink, since it's not supported yet. And it's also a kinda workaround for bug 748681
Review of attachment 317410 [details] [review]: ::: sys/directsound/gstdirectsoundsrc.c @@ +87,3 @@ { PROP_0, PROP_DEVICE_NAME, Just remove the device-name property, it doesn't really make sense. There could also be multiple devices with the same name (but different GUID). Having only one property also makes the code easier.
yes and no - the use case for device-name is that a user can change/configure it in windows. And as well the mixer settings (volume, name) is based on device-name.
The mixer is not related to a device's GUID?
As it is done its a string compare to the device-name to find a matching volume control. It's because it uses an old mixer api. Nowadays you usually use Endpoint Api which support selecting an mixer control via guid or device. Which means enabling mixer support by guid would mean an backward search for device-name by device-guid, ugly...
Ok, let's go with this then. Only problem is that it doesn't apply cleanly to git master :) Can you update?
Created attachment 317506 [details] [review] add guid device property as it is done in directsoundsink
Created attachment 317507 [details] [review] add guid device property as it is done in directsoundsink
Review of attachment 317507 [details] [review]: ::: sys/directsound/gstdirectsoundsrc.c @@ +429,3 @@ + /* Create capture object */ + hRes = + pDSoundCaptureCreate (dsoundsrc->device_guid, &dsoundsrc->pDSC, NULL); You have this line twice here now @@ +437,3 @@ + // mixer is only supported when device-id is not set + if (!dsoundsrc->device_id) { + gst_directsound_src_mixer_init (dsoundsrc); Or maybe we should just remove the mixer support completely here? ::: sys/directsound/gstdirectsoundsrc.h @@ +65,3 @@ #define GST_IS_DIRECTSOUND_SRC(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj),GST_TYPE_DIRECTSOUND_SRC)) #define GST_IS_DIRECTSOUND_SRC_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass),GST_TYPE_DIRECTSOUND_SRC)) +typedef struct _GstDirectSoundSrc GstDirectSoundSrc; Don't run gst-indent on headers, only on .c files
Created attachment 317609 [details] [review] add guid device property as it is done in directsoundsink - removed the duplicate init function call - add msg that mixer will not work when device is selected by guid - gst-indent only on c file ;)
Comment on attachment 317609 [details] [review] add guid device property as it is done in directsoundsink Looks good but does not apply to GIT master :) Can you update?
It might also make sense to just remove the mixer functionality here. The sink also does not have it and its usefulness seems limited.
I think the mixer functionality will work if you have the right encoding set on your windows and set the device by device-name. So it think just keep it and inform the user that it will not work with device set by guid.
Created attachment 317622 [details] [review] add guid device property as it is done in directsoundsink if it again not applies to master, please tell me what I'm doing wrong. made an "git reset --hard origin/master" and merged my changes, did gst-indent on c file, git commit, git format-patch -1 ->?
This patch looks incomplete, it only has the header changes and a comment change in the .c file. Not sure how that could've happened. The only thing you need to ensure is that the top commit in your branch has all the changes you want to commit, then "git format-patch -1" will give you a patch with all the changes. Or -2 if the top two commits, etc.
Created attachment 317624 [details] [review] add guid device property as it is done in directsoundsink tried again to generate the patch, please check again
Problem was that you had the header already converted to UNIX line endings. All good now :) commit f9464ce3549c2b3948a582464631ba4f8dd0d5e7 Author: Thomas Roos <thomas.roos@industronic.de> Date: Fri Dec 18 12:26:16 2015 +0100 directsoundsrc: add device property as it is done in directsoundsink This allows selection of the device by GUID instead of the name. The name is user-given and multiple devices can have the same name. https://bugzilla.gnome.org/show_bug.cgi?id=759484 commit d4920948aa4ccd7fc76a2266223eed7cfb013a71 Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Dec 18 12:36:26 2015 +0100 directsoundsrc: Convert header from (some) DOS line endings to UNIX A mix between different line endings in the same file is not a good idea, and the .c files are both with UNIX line endings so let's use that.