Bug 40636 - Change public facing references to "sync_volumes"
Summary: Change public facing references to "sync_volumes"
Status: RESOLVED FIXED
Alias: None
Product: PulseAudio
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Colin Guthrie
QA Contact: pulseaudio-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 40193
  Show dependency treegraph
 
Reported: 2011-09-05 02:26 UTC by Colin Guthrie
Modified: 2011-09-14 01:25 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to rename things. (52.87 KB, patch)
2011-09-13 13:26 UTC, Colin Guthrie
Details | Splinter Review

Description Colin Guthrie 2011-09-05 02:26:22 UTC
Lennart would prefer a better name (in external references at least - i.e. config files, profile files, man pages, module arguments and APIs) for sync_volumes. Internal references can stay for now but probably should reflect the "public name" eventually (but perhaps after 1.0!)

His comments:
"I don't like the "sync_volume" name. It's too generic and could mean everything. I know it's hard coming up with good names for this kind of stuff and it's not always possible to find a name that is self-explanatory but "sync_volume" is just bad I think."

So any suggestions?
Comment 1 Tanu Kaskinen 2011-09-05 09:43:09 UTC
(In reply to comment #0)
> Lennart would prefer a better name (in external references at least - i.e.
> config files, profile files, man pages, module arguments and APIs) for
> sync_volumes. Internal references can stay for now but probably should reflect
> the "public name" eventually (but perhaps after 1.0!)

I guess "APIs" means the sink flag? Or is it visible elsewhere? I don't really like having that flag there in the public API at all. It's not useful for clients.

> His comments:
> "I don't like the "sync_volume" name. It's too generic and could mean
> everything. I know it's hard coming up with good names for this kind of stuff
> and it's not always possible to find a name that is self-explanatory but
> "sync_volume" is just bad I think."
> 
> So any suggestions?

No. I don't agree that "sync_volume" is "just bad". The current name is pretty generic, but the only problem is that the user has to read some documentation to understand it. Not a big deal IMO. If someone comes up with a better name, then that's obviously great, but as long as there are zero suggestions, I don't think this should be a 1.0 blocker.
Comment 2 David Henningsson 2011-09-07 03:08:06 UTC
This is all about the stuff that's suppose to avoid the loudness bumps, right?

The best I could come up with was "mixer_margin", that seems somewhat more specific?
Comment 3 Tanu Kaskinen 2011-09-07 13:01:00 UTC
(In reply to comment #2)
> This is all about the stuff that's suppose to avoid the loudness bumps, right?
> 
> The best I could come up with was "mixer_margin", that seems somewhat more
> specific?

I don't really like that - it seems to me that "mixer" is pretty specific to the alsa terminology (it's not used in pulseaudio in other contexts). And there's "sync volume safety margin" - would that become "mixer margin safety margin"?

I got now a competing proposal: "delayed hw volume".

The feature is really all about "synchronized hw and sw volume changes", so I still think "sync volume" is the best idea so far (no, it's not my invention), but the mechanism through which the synchronization is achieved is delayed hw volume changes, and thus "delayed hw volume" might be an appropriate name.
Comment 4 Arun Raghavan 2011-09-11 06:27:10 UTC
I see Lennart's point about that sync_volumes not conveying sufficient information. He seemed okay with "deferred volumes", so either deferred or delayed (hardware) volumes sounds okay to me.
Comment 5 Colin Guthrie 2011-09-13 13:26:39 UTC
Created attachment 51164 [details] [review]
Patch to rename things.

OK, on a clean clone, I just did:

grep -rli "sync[-_]volume" . | xargs sed -i 's/sync_volume/deferred_volume/g;s/PA_SINK_SYNC_VOLUME/PA_SINK_DEFERRED_VOLUME/g;s/PA_SOURCE_SYNC_VOLUME/PA_SOURCE_DEFERRED_VOLUME/g;s/sync-volume/deferred-volume/g'


This leaves: 

[colin@jimmy src (master)]$ ccgrep SYNC_VOLUME .
./pulsecore/source.c:        case PA_SOURCE_MESSAGE_SYNC_VOLUMES:
./pulsecore/sink.h:    PA_SINK_MESSAGE_SYNC_VOLUMES,
./pulsecore/sink.c:        case PA_SINK_MESSAGE_SYNC_VOLUMES:
./pulsecore/source.h:    PA_SOURCE_MESSAGE_SYNC_VOLUMES,


Which I think I want to keep as the message name describes the action which I think is valid.

The attached patch implements this and passes a distcheck.

If no one spots anything obvious I'll commit this tomorrow morning.
Comment 6 Colin Guthrie 2011-09-14 01:25:55 UTC
commit aa3142ab208000fe675b1deba46461e7fe8e470f
Author: Colin Guthrie <colin@mageia.org>
Date:   Tue Sep 13 21:15:49 2011 +0100

    volume: Rename 'sync volume' to 'deferred volume'.
    
    This just covers Lennart's concern over the terminology used.
    
    The majority of this change is simply the following command:
     grep -rli sync[-_]volume . | xargs sed -i 's/sync_volume/deferred_volume/g;s/PA_SINK_SYNC_VOLUME/PA_SINK_DEFERRED_VOLUME/g;s/PA_SOURCE_SYNC_VOLUME/PA_SOURCE_DEFERRED_VOLUME/g;s/sync-volume/deferred-volume/g'
    
    Some minor tweaks were added on top to tidy up formatting and
    a couple of phrases were clarified too.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.