pulse-server: improve reference counting of samples

Previously, when a sample is "committed" from an upload stream,
its reference count is set to 1. This is problematic if,
when the sample is committed for a second time, there are
streams playing the sample as the reference count will go out
of sync.

The problem can be easily triggered, especially with longer samples:

   pactl upload-sample a-long-sample.ogg
   pactl play-sample a-long-sample
   pactl play-sample a-long-sample
   pactl upload-sample a-long-sample.ogg  # while playing

When the first stream finishes playing, it will free the sample,
which can cause problems e.g. for the second stream playing the
sample at that very moment.

Fix that by decoupling the buffer from the sample and making
the buffer reference counted. And remove the reference counting
from the samples as it is no longer needed.

Futhermore, previously, the reference counts were ignored
when the removal of a sample was requested. That is fixed
as well.

The previous issue can be triggered easily as well:

  pactl upload-sample a-long-sample.ogg
  pactl play-sample a-long-sample
  pactl remove-sample a-long-sample      # while playing

Fixes #1953

src/modules/module-protocol-pulse/pulse-server.c

@@ -2110,7 +2110,7 @@ static int do_finish_upload_stream(struct client *client, uint32_t command, uint
 {
 	struct impl *impl = client->impl;
 	uint32_t channel, event;
-	struct stream *stream = NULL;
+	struct stream *stream;
 	struct sample *sample;
 	const char *name;
 	int res;
@@ -2134,22 +2134,36 @@ static int do_finish_upload_stream(struct client *client, uint32_t command, uint
 			client->name, commands[command].name, tag,
 			channel, name);
 
-	sample = find_sample(impl, SPA_ID_INVALID, name);
-	if (sample == NULL) {
-		sample = calloc(1, sizeof(struct sample));
+	struct sample *old = find_sample(impl, SPA_ID_INVALID, name);
+	if (old == NULL || (old != NULL && old->ref > 1)) {
+		sample = calloc(1, sizeof(*sample));
 		if (sample == NULL)
 			goto error_errno;
 
-		sample->index = pw_map_insert_new(&impl->samples, sample);
-		if (sample->index == SPA_ID_INVALID)
-			goto error_errno;
+		if (old != NULL) {
+			sample->index = old->index;
+			spa_assert_se(pw_map_insert_at(&impl->samples, sample->index, sample) == 0);
 
-		event = SUBSCRIPTION_EVENT_NEW;
+			old->index = SPA_ID_INVALID;
+			sample_unref(old);
+		} else {
+			sample->index = pw_map_insert_new(&impl->samples, sample);
+			if (sample->index == SPA_ID_INVALID)
+				goto error_errno;
+		}
 	} else {
-		pw_properties_free(sample->props);
-		free(sample->buffer);
-		event = SUBSCRIPTION_EVENT_CHANGE;
+		pw_properties_free(old->props);
+		free(old->buffer);
+		impl->stat.sample_cache -= old->length;
+
+		sample = old;
 	}
+
+	if (old != NULL)
+		event = SUBSCRIPTION_EVENT_CHANGE;
+	else
+		event = SUBSCRIPTION_EVENT_NEW;
+
 	sample->ref = 1;
 	sample->impl = impl;
 	sample->name = name;
@@ -2442,7 +2456,10 @@ static int do_remove_sample(struct client *client, uint32_t command, uint32_t ta
 			SUBSCRIPTION_EVENT_SAMPLE_CACHE,
 			sample->index);
 
-	sample_free(sample);
+	pw_map_remove(&impl->samples, sample->index);
+	sample->index = SPA_ID_INVALID;
+
+	sample_unref(sample);
 
 	return reply_simple_ack(client, tag);
 }

src/modules/module-protocol-pulse/sample-play.c

@@ -77,12 +77,11 @@ static void sample_play_stream_destroy(void *data)
 	struct sample_play *p = data;
 
 	pw_log_info("destroy %s", p->sample->name);
+
 	spa_hook_remove(&p->listener);
-
-	if (--p->sample->ref == 0)
-		sample_free(p->sample);
-
 	p->stream = NULL;
+
+	sample_unref(p->sample);
 	p->sample = NULL;
 }
 
@@ -173,9 +172,11 @@ struct sample_play *sample_play_new(struct pw_core *core,
 		goto error_free;
 	}
 
-	p->sample = sample;
+	/* safe to increment the reference count here because it will be decreased
+	   by the stream's 'destroy' event handler, which will be called
+	   (even if `pw_stream_connect()` fails) */
+	p->sample = sample_ref(sample);
 	p->stride = sample_spec_frame_size(&sample->ss);
-	sample->ref++;
 
 	pw_stream_add_listener(p->stream,
 			&p->listener,

src/modules/module-protocol-pulse/sample.h

@@ -46,4 +46,16 @@ struct sample {
 
 void sample_free(struct sample *sample);
 
+static inline struct sample *sample_ref(struct sample *sample)
+{
+	sample->ref++;
+	return sample;
+}
+
+static inline void sample_unref(struct sample *sample)
+{
+	if (--sample->ref == 0)
+		sample_free(sample);
+}
+
 #endif /* PULSE_SERVER_SAMPLE_H */