From 0f583e348794b45e4307e70a29865faddcba407f Mon Sep 17 00:00:00 2001 From: Andrew Opalach Date: Mon, 7 Apr 2025 10:48:59 -0400 Subject: Fix hold in local sink, improve seek behavior - Make sure previous queue is ran on single frames. Signed-off-by: Andrew Opalach --- src/libsink/sink.c | 123 ++++++++++++++++++++++++++++------------------------- src/libsink/sink.h | 1 + 2 files changed, 65 insertions(+), 59 deletions(-) (limited to 'src/libsink') diff --git a/src/libsink/sink.c b/src/libsink/sink.c index 5bf9597..fe87df5 100644 --- a/src/libsink/sink.c +++ b/src/libsink/sink.c @@ -208,7 +208,7 @@ static void remove_entry_video_buffer(struct camu_sink_entry *entry) // BUFFER_ENDED: No-op'd in remove_entry_buffers() and add_or_queue_entry() but otherwise unchanged. static void remove_entry_buffers(struct camu_sink_entry *entry) { - trace("remove_entry_buffers("ENTRY_FMT"), audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), AUDIO_STATE(entry), VIDEO_STATE(entry)); + log_trace("remove_entry_buffers("ENTRY_FMT"), audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), AUDIO_STATE(entry), VIDEO_STATE(entry)); al_assert(!entry->ended); if (AUDIO_STATE(entry) != BUFFER_ENDED) { remove_entry_audio_buffer(entry); @@ -223,7 +223,7 @@ static void add_video_if_set_and_buffered(struct camu_sink_entry *entry); static void add_or_queue_entry(struct camu_sink_entry *entry) { - trace("add_or_queue_entry("ENTRY_FMT"), audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), AUDIO_STATE(entry), VIDEO_STATE(entry)); + log_trace("add_or_queue_entry("ENTRY_FMT"), audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), AUDIO_STATE(entry), VIDEO_STATE(entry)); al_assert(AUDIO_STATE(entry) != BUFFER_QUEUED); al_assert(VIDEO_STATE(entry) != BUFFER_QUEUED); if (AUDIO_STATE(entry) == BUFFER_INIT) { @@ -251,6 +251,7 @@ static void maybe_disconnect_entry(struct camu_sink_entry *entry) #ifdef CAMU_SINK_LOCAL static void sink_local_pause(struct camu_sink *sink, struct camu_sink_entry *entry) { + if (entry->held) return; if (!camu_clock_is_paused(&entry->clock)) { entry->paused = true; camu_clock_pause(&entry->clock, 0); @@ -466,7 +467,7 @@ static void mixer_callback(void *userdata, u8 op) { struct camu_sink *sink = (struct camu_sink *)userdata; if (op == CAMU_MIXER_EMPTY) { - info("Mixer empty."); + log_info("Mixer empty."); #ifndef LIANA_LIST_SCUFFED_LOOP queue_cmd(sink, (struct camu_sink_cmd){ .op = STOP, @@ -548,7 +549,7 @@ static void maybe_cleanup_old_entries(struct camu_sink *sink) static void maybe_remove_previous(struct camu_sink *sink) { - trace("maybe_remove_previous(), previous_count: %u.", sink->previous.count); + log_trace("maybe_remove_previous(), previous_count: %u.", sink->previous.count); struct camu_sink_entry *previous; al_array_foreach(sink->previous, i, previous) { remove_entry_buffers(previous); @@ -569,14 +570,14 @@ static void remove_previous_if_contains(struct camu_sink *sink, struct camu_sink break; } } - trace("remove_previous_if_contains("ENTRY_FMT"), removed: %s.", ENTRY_ARG(key), BOOLSTR(removed)); + log_trace("remove_previous_if_contains("ENTRY_FMT"), removed: %s.", ENTRY_ARG(key), BOOLSTR(removed)); } // Every call to maybe_add_to_previous() must map to a remove_entry_buffers(). static void maybe_add_to_previous(struct camu_sink *sink, struct camu_sink_entry *previous, struct camu_sink_entry *target) { - trace("maybe_add_to_previous("ENTRY_FMT", "ENTRY_FMT").", ENTRY_ARG(previous), ENTRY_ARG(target)); + log_trace("maybe_add_to_previous("ENTRY_FMT", "ENTRY_FMT").", ENTRY_ARG(previous), ENTRY_ARG(target)); al_assert(previous != target); al_assert(!previous->ended); // If none of the entry's buffers are added, we don't care about adding it to previous. @@ -649,6 +650,7 @@ void add_video_if_set_and_buffered(struct camu_sink_entry *entry) VIDEO_STATE(entry) = BUFFER_ADDED; if (VIDEO_IS_SINGLE_FRAME(entry)) { add_entry_video_buffer(entry); + if (AUDIO_EMPTY(entry)) maybe_remove_previous(entry->sink); } else if (AUDIO_ADDED_OR_EMPTY(entry)) { do_add_entry(entry); } @@ -658,7 +660,7 @@ void add_video_if_set_and_buffered(struct camu_sink_entry *entry) static void switch_to(struct camu_sink *sink, struct camu_sink_entry *target) { - trace("switch_to("ENTRY_FMT"), current: "ENTRY_FMT".", ENTRY_ARG(target), ENTRY_ARG(sink->current)); + log_trace("switch_to("ENTRY_FMT"), current: "ENTRY_FMT".", ENTRY_ARG(target), ENTRY_ARG(sink->current)); bool ensure_removed = false; struct camu_sink_entry *current = sink->current; @@ -669,7 +671,7 @@ static void switch_to(struct camu_sink *sink, struct camu_sink_entry *target) if (detached) { al_assert(detached == current); sink->detached = NULL; - warn("Unset detached as a substitute for remove."); + log_warn("Unset detached as a substitute for remove."); // This should only matter if detached was unconfigured. if (AUDIO_EMPTY(detached)) AUDIO_STATE(detached) = BUFFER_INIT; if (VIDEO_EMPTY(detached)) VIDEO_STATE(detached) = BUFFER_INIT; @@ -701,14 +703,14 @@ static void switch_to(struct camu_sink *sink, struct camu_sink_entry *target) } if (!dangling_target) { - sink->current = target; - sink->current->audio.ignore_paused = false; - if (!sink->current->paused) { - camu_audio_buffer_resync(&sink->current->audio.buf); + target->audio.ignore_paused = false; + if (!target->paused) { + camu_audio_buffer_resync(&target->audio.buf); } + sink->current = target; } else { - sink->current = NULL; stop_video = true; + sink->current = NULL; } if (stop_video) { @@ -724,22 +726,25 @@ static void switch_to(struct camu_sink *sink, struct camu_sink_entry *target) static void pause_and_swap_to(struct camu_sink *sink, struct camu_sink_entry *target, u64 at) { struct camu_sink_entry *current = sink->current; - trace("pause_and_swap_to("ENTRY_FMT"), current: "ENTRY_FMT".", ENTRY_ARG(target), ENTRY_ARG(current)); + log_trace("pause_and_swap_to("ENTRY_FMT"), current: "ENTRY_FMT".", ENTRY_ARG(target), ENTRY_ARG(current)); al_assert(target != current); + al_assert(!sink->target); + // This is extra verbose because the order is important. + // 1. sink->target has to be set before calling clock_pause(). + // 2. current must still be paused even if it's ended. + // 3. In the immediate case, switch_to() has to come last. + if (current && !current->ended) sink->target = target; if (current) { - if (!current->ended) sink->target = target; current->audio.ignore_paused = true; camu_clock_pause(¤t->clock, at); } - if (!current || current->ended) { - switch_to(sink, target); - } + if (!current || current->ended) switch_to(sink, target); } #endif static bool end_entry_and_advance_queue(struct camu_sink *sink, struct camu_sink_entry *entry) { - info("Entry ("ENTRY_FMT") ended.", ENTRY_ARG(entry)); + log_info("Entry ("ENTRY_FMT") ended.", ENTRY_ARG(entry)); remove_previous_if_contains(sink, entry); #ifdef CAMU_SINK_ONESHOT sink->callback(sink->userdata, CAMU_SINK_MOCK_CLOSE, 0, NULL); @@ -753,7 +758,7 @@ static bool end_entry_and_advance_queue(struct camu_sink *sink, struct camu_sink .opaque = entry }); #ifdef LIANA_LIST_SCUFFED_LOOP - info("Looping."); + log_info("Looping."); return true; #endif if (sink->target) { @@ -789,7 +794,7 @@ static void audio_buffer_callback(void *userdata, u8 op) case CAMU_BUFFER_PAUSED: nn_mutex_lock(&sink->lock); if (!entry->audio.ignore_paused && entry->paused) { - info("Audio buffer paused."); + log_info("Audio buffer paused."); queue_cmd(sink, (struct camu_sink_cmd){ .op = STOP, .value.i = CAMU_SINK_AUDIO @@ -797,12 +802,12 @@ static void audio_buffer_callback(void *userdata, u8 op) } nn_mutex_unlock(&sink->lock); break; - case CAMU_BUFFER_EOF: { - info("Audio EOF."); + case CAMU_BUFFER_EOF: + log_info("Audio EOF."); nn_mutex_lock(&sink->lock); - // This buffer's state could be ADDED, SET_OR_BUFFERED, or CONFIGURED. - // Having threaded outputs means anything could have happened while waiting on the lock. - // If we were waiting in CLIENT_REMOVE_BUFFERS, state could very well be CONFIGURED here. + // Having threaded outputs means anything could have happened while waiting + // on the lock above. If we were locked in CLIENT_REMOVE_BUFFERS, state could very + // well be CONFIGURED here. if (AUDIO_STATE(entry) == BUFFER_ADDED) { remove_entry_audio_buffer(entry); } @@ -812,9 +817,8 @@ static void audio_buffer_callback(void *userdata, u8 op) } nn_mutex_unlock(&sink->lock); break; - } case CAMU_BUFFER_ERRORED: - error("Audio buffer errored."); + log_error("Audio buffer errored."); queue_cmd(sink, (struct camu_sink_cmd){ .op = EJECT_ENTRY, .opaque = entry @@ -840,10 +844,10 @@ static void video_buffer_callback(void *userdata, u8 op) case CAMU_BUFFER_UNCORK: lia_vcr_uncork(entry->video.track); break; - case CAMU_BUFFER_EOF: { - info("Video EOF."); - bool swapped = false; + case CAMU_BUFFER_EOF: + log_info("Video EOF."); bool single_frame = VIDEO_IS_SINGLE_FRAME(entry); + bool swapped = false; nn_mutex_lock(&sink->lock); // This buffer's state could be ADDED, SET_OR_BUFFERED, or CONFIGURED. if (!AUDIO_EMPTY(entry)) { @@ -866,9 +870,8 @@ static void video_buffer_callback(void *userdata, u8 op) }); } break; - } case CAMU_BUFFER_ERRORED: - error("Video buffer errored."); + log_error("Video buffer errored."); queue_cmd(sink, (struct camu_sink_cmd){ .op = EJECT_ENTRY, .opaque = entry @@ -883,7 +886,7 @@ static void clock_callback(void *userdata, u8 op) struct camu_sink *sink = entry->sink; if (op == CAMU_CLOCK_PAUSED) { nn_mutex_lock(&sink->lock); - trace("clock_paused("ENTRY_FMT"), target: "ENTRY_FMT".", ENTRY_ARG(entry), ENTRY_ARG(sink->target)); + log_trace("clock_paused("ENTRY_FMT"), target: "ENTRY_FMT".", ENTRY_ARG(entry), ENTRY_ARG(sink->target)); if (entry == sink->current) { if (sink->target) { switch_to(sink, sink->target); @@ -965,7 +968,7 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str case CAMU_STREAM_AUDIO: entry->audio.track = (struct lia_vcr_track *)opaque; if (!camu_audio_buffer_configure(&entry->audio.buf, stream, sink->audio.mixer)) { - error("Audio buffer failed to configure."); + log_error("Audio buffer failed to configure."); maybe_disconnect_entry(entry); return; } @@ -981,7 +984,7 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str case CAMU_STREAM_VIDEO: entry->video.track = (struct lia_vcr_track *)opaque; if (!camu_video_buffer_configure(&entry->video.buf, stream, sink->video.renderer)) { - error("Video buffer failed to configure."); + log_error("Video buffer failed to configure."); maybe_disconnect_entry(entry); return; } @@ -996,7 +999,7 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str break; case CAMU_STREAM_SUBTITLE: if (!camu_video_buffer_configure_subtitles(&entry->video.buf, stream)) { - warn("Video buffer failed to configure subtitles."); + log_warn("Video buffer failed to configure subtitles."); } break; case CAMU_STREAM_ATTACHMENT: { @@ -1043,7 +1046,7 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str case LIANA_CLIENT_REMOVE_BUFFERS: { struct lia_reconnect_info *rec = (struct lia_reconnect_info *)opaque; nn_mutex_lock(&sink->lock); - trace("remove_buffers("ENTRY_FMT", %s, %s), entry == current: %s.", ENTRY_ARG(entry), BOOLSTR(rec->reconnect), BOOLSTR(rec->unconfigured), BOOLSTR(entry == sink->current)); + log_trace("remove_buffers("ENTRY_FMT", %s, %s), entry == current: %s.", ENTRY_ARG(entry), BOOLSTR(rec->reconnect), BOOLSTR(rec->unconfigured), BOOLSTR(entry == sink->current)); // This entry might be in previous if it was added to previous then, // 1. it's being cleaned up after ENTRY_MAX_AGE - 1 entries were added but none buffered. @@ -1108,17 +1111,18 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str nn_mutex_unlock(&sink->lock); - // This buffer won't be re-added until after a CLIENT_RECONNECTED event. - if (rec->reconnect) { - if (!AUDIO_EMPTY(entry)) camu_audio_buffer_reset(&entry->audio.buf); - if (!ignore_video && !VIDEO_EMPTY(entry)) camu_video_buffer_reset(&entry->video.buf); - } - break; } case LIANA_CLIENT_RESUME_AT: { struct lia_timing *time = (struct lia_timing *)opaque; - trace("resume_at("ENTRY_FMT"), paused_at: %f.", ENTRY_ARG(entry), entry->clock.paused_at); + log_trace("resume_at("ENTRY_FMT"), paused_at: %f.", ENTRY_ARG(entry), entry->clock.paused_at); + // These buffers won't be re-added until after a CLIENT_RECONNECTED event. + if (!AUDIO_EMPTY(entry)) { + camu_audio_buffer_reset(&entry->audio.buf); + } + if (!VIDEO_EMPTY(entry) && !VIDEO_IS_SINGLE_FRAME(entry)) { + camu_video_buffer_reset(&entry->video.buf, time->seek_pos); + } #ifdef CAMU_SINK_LOCAL time->at = 0; #endif @@ -1138,7 +1142,7 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str case LIANA_CLIENT_RECONNECTED: { struct lia_reconnect_info *rec = (struct lia_reconnect_info *)opaque; nn_mutex_lock(&sink->lock); - trace("reconnected("ENTRY_FMT"), detached: "ENTRY_FMT", audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), ENTRY_ARG(sink->detached), AUDIO_STATE(entry), VIDEO_STATE(entry)); + log_trace("reconnected("ENTRY_FMT"), detached: "ENTRY_FMT", audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), ENTRY_ARG(sink->detached), AUDIO_STATE(entry), VIDEO_STATE(entry)); if (entry == sink->detached) { al_assert(entry == sink->current); al_assert(AUDIO_STATE(entry) != BUFFER_ENDED); @@ -1197,7 +1201,7 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str if (entry == sink->target) { sink->target = (struct camu_sink_entry *)0xb00b; - warn("Attempting to handle a disconnected target."); + log_warn("Attempting to handle a disconnected target."); } else if (entry == sink->current) { if (sink->detached) { al_assert(sink->detached == sink->current); @@ -1226,7 +1230,7 @@ static void client_callback(void *userdata, u8 op, struct camu_codec_stream *str lia_client_free(&entry->client); camu_audio_buffer_free(&entry->audio.buf); camu_video_buffer_free(&entry->video.buf); - info("Entry ("ENTRY_FMT") closed by %s.", ENTRY_ARG(entry), removed ? "force" : "cleanup"); + log_info("Entry ("ENTRY_FMT") closed by %s.", ENTRY_ARG(entry), removed ? "force" : "cleanup"); al_free(entry); break; @@ -1243,7 +1247,7 @@ static struct camu_sink_entry *create_entry(struct camu_sink *sink, u32 id) entry->ended = false; camu_clock_init(&entry->clock, clock_callback, entry); - entry->paused = false; + entry->held = false; entry->client.callback = client_callback; entry->client.userdata = entry; @@ -1336,24 +1340,25 @@ static bool set_command_callback(void *userdata, struct nn_rpc_connection *conn, #ifdef CAMU_SINK_LOCAL (void)at; al_assert(entry != current); - trace("set("ENTRY_FMT"), %s[local], created: %s.", ENTRY_ARG(entry), lia_pause_op_name(pause), BOOLSTR(create)); - if (current && !current->ended) { + log_trace("set("ENTRY_FMT"), %s[local], created: %s.", ENTRY_ARG(entry), lia_pause_op_name(pause), BOOLSTR(create)); + if (current) { current->audio.ignore_paused = true; if (!current->paused) { + // paused has to map directly to the clock state here. + al_assert(!camu_clock_is_paused(¤t->clock)); camu_clock_pause(¤t->clock, 0); } + current->held = true; } - if (!entry->ended) { - entry->audio.ignore_paused = false; - if (!entry->paused) { - camu_clock_resume(&entry->clock, 0); - } + entry->held = false; + if (!entry->paused) { + camu_clock_resume(&entry->clock, 0); } switch_to(sink, entry); #else // For target to be set that must mean current is set, armed to pause, and not ended. struct camu_sink_entry *prev_target = sink->target; - trace("set("ENTRY_FMT"), %s, created: %s, target: "ENTRY_FMT".", ENTRY_ARG(entry), lia_pause_op_name(pause), BOOLSTR(create), ENTRY_ARG(prev_target)); + log_trace("set("ENTRY_FMT"), %s, created: %s, target: "ENTRY_FMT".", ENTRY_ARG(entry), lia_pause_op_name(pause), BOOLSTR(create), ENTRY_ARG(prev_target)); switch (pause) { case LIANA_PAUSE_NONE: if (prev_target) { @@ -1445,7 +1450,7 @@ static bool pause_command_callback(void *userdata, struct nn_rpc_connection *con if (!entry) goto out; al_assert(entry->sequence == sequence); nn_mutex_lock(&sink->lock); - trace("pause("ENTRY_FMT"), %s, audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), lia_pause_op_name(pause), AUDIO_STATE(entry), VIDEO_STATE(entry)); + log_trace("pause("ENTRY_FMT"), %s, audio_state: %hhu, video_state: %hhu.", ENTRY_ARG(entry), lia_pause_op_name(pause), AUDIO_STATE(entry), VIDEO_STATE(entry)); #ifdef CAMU_SINK_LOCAL (void)at; sink_local_pause(sink, entry); @@ -1499,7 +1504,7 @@ static bool seek_command_callback(void *userdata, struct nn_rpc_connection *conn struct camu_sink_entry *entry = get_entry_from_id(sink, id); if (!entry) goto out; - trace("seek("ENTRY_FMT"), reset_id: %u.", ENTRY_ARG(entry), reset_id); + log_trace("seek("ENTRY_FMT"), reset_id: %u.", ENTRY_ARG(entry), reset_id); entry->reset_id = reset_id; // The rest of the seek is handled in CLIENT_REMOVE_BUFFERS/RESUME_AT/RECONNECTED. lia_client_seek(&entry->client, pos, at); diff --git a/src/libsink/sink.h b/src/libsink/sink.h index ba37241..c2ea6a1 100644 --- a/src/libsink/sink.h +++ b/src/libsink/sink.h @@ -51,6 +51,7 @@ struct camu_sink_entry { bool ended; u32 reset_id; struct camu_clock clock; + bool held; // Only used in SINK_LOCAL. bool paused; struct { u8 state; -- cgit v1.2.3-101-g0448