[pulseaudio-discuss] [PATCH 3/3] devicewidget: Add a latency offset spinbutton

[Tanu Kaskinen]

On Sun, 2012-07-22 at 15:23 +0200, poljar (Damir Jelić) wrote:
> @@ -133,6 +160,14 @@ bool DeviceWidget::timeoutEvent() {
>  void DeviceWidget::executeVolumeUpdate() {
>  }
>  
> +void DeviceWidget::updatePorts(std::map<Glib::ustring, PortInfo> &ports) {
> +    if (pa_context_get_server_protocol_version(get_context()) >= 27) {
> +        offsetButtonEnabled = false;
> +        offsetButton->set_value(ports.find(activePort)->second.latency_offset / 1000.0);

I feel uneasy about the assumption that ports.find(activePort) will find
something. The main problem is cards without ports, but I also think we
shouldn't crash if the server sends a bogus port list. And I don't like
the protocol version check, although it probably avoids quite
effectively the risk of cards without ports. IMO, applications should
avoid caring about the protocol version.

I'd change the updatePorts() function to setLatencyOffset(), which takes
only the new offset as the parameter.

> @@ -157,10 +192,18 @@ void DeviceWidget::prepareMenu() {
>      if (active_idx >= 0)
>          portList->set_active(active_idx);
>  
> -    if (ports.size() > 0)
> +    if (ports.size() > 0) {
>          portSelect->show();
> -    else
> +
> +        if (pa_context_get_server_protocol_version(get_context()) >= 27)
> +            offsetSelect->show();
> +        else
> +            offsetSelect->hide();

Hmm, this protocol version check seems to be well placed, because there
isn't any better way to check if the server supports latency offsets,
and it's a good idea to hide the offset from the UI if the feature isn't
supported.

> @@ -367,6 +398,8 @@ bool MainWindow::updateSink(const pa_sink_info &info) {
>  
>      w->activePort = info.active_port ? info.active_port->name : "";
>  
> +    w->updatePorts(cardWidgets[info.card]->ports);
> +
>  #ifdef PA_SINK_SET_FORMATS
>      w->setDigital(info.flags & PA_SINK_SET_FORMATS);
>  #endif
> @@ -516,6 +549,8 @@ void MainWindow::updateSource(const pa_source_info &info) {
>  
>      w->activePort = info.active_port ? info.active_port->name : "";
>  
> +    w->updatePorts(cardWidgets[info.card]->ports);
> +

This crashes if the server sends bogus sink/source information (that is,
if cardWidgets doesn't contain info.card).

-- 
Tanu