[vlc-commits] [Git][videolan/vlc][master] 4 commits: aaudio: add a lock when flushing

[Steve Lhomme (@robUx4)]

Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
4b8fd6c6 by Thomas Guillem at 2023-03-09T09:34:21+00:00
aaudio: add a lock when flushing

The doc says : "Pausing a stream will freeze the data flow but not flush
any buffers".

So I guess this is false for some devices like the Galaxy S22 Ultra (API
level 31): the data callback can still be called when paused. This was
resulting on use-after-free or latency gap.

- - - - -
4411fe1b by Thomas Guillem at 2023-03-09T09:34:21+00:00
aaudio: clip pos_frames

Instead of asserting it.

There might some (very small) imprecision with
`AAudioStream_getTimestamp` when starting the stream.

- - - - -
07bbc7ba by Thomas Guillem at 2023-03-09T09:34:21+00:00
aaudio: reset flush pos in case of error

- - - - -
a2a38785 by Thomas Guillem at 2023-03-09T09:34:21+00:00
aaudio: don't fail silently

`AAudioStream_getTimestamp` can fail and we should log it since it's
quite critical (no A/V sync in such case).

(Yes, it can happen and I don't have the solution yet)

- - - - -


1 changed file:

- modules/audio_output/android/aaudio.c


Changes:

=====================================
modules/audio_output/android/aaudio.c
=====================================
@@ -329,7 +329,12 @@ GetFrameTimestampLocked(aout_stream_t *stream, int64_t *pos_frames,
     aaudio_result_t result =
             vt.AAudioStream_getTimestamp(sys->as, CLOCK_MONOTONIC,
                                          pos_frames, &time_ns);
-    if (result != AAUDIO_OK || *pos_frames <= 0)
+    if (result != AAUDIO_OK)
+    {
+        msg_Err(stream, "AAudioStream_getTimestamp failed: %d", result);
+        return VLC_EGENERIC;
+    }
+    if (*pos_frames <= 0)
         return VLC_EGENERIC;
 
     *frame_time_us = VLC_TICK_FROM_NS(time_ns);
@@ -424,7 +429,8 @@ DataCallback(AAudioStream *as, void *user, void *data_, int32_t num_frames)
                 TicksToBytes(sys, TIMING_REPORT_DELAY_TICKS);
 
             pos_frames -= sys->frames_flush_pos;
-            assert(pos_frames > 0);
+            if (unlikely(pos_frames < 0))
+                pos_frames = 0;
             vlc_tick_t pos_ticks = FramesToTicks(sys, pos_frames);
 
             /* Add the start silence to the system time and don't subtract
@@ -571,7 +577,13 @@ Flush(aout_stream_t *stream)
      && WaitState(stream, AAUDIO_STREAM_STATE_PAUSED) != VLC_SUCCESS)
         return;
 
-    /* No lock needed since the DataCallback stop being called (paused) */
+    /* The doc states that "Pausing a stream will freeze the data flow but not
+     * flush any buffers" but this does not seem to be the case for all
+     * arch/devices/versions. So, flush everything with the lock held in the
+     * unlikely case that the data callback is called between PAUSED and
+     * FLUSHING */
+    vlc_mutex_lock(&sys->lock);
+
     vlc_frame_ChainRelease(sys->frame_chain);
     sys->frame_chain = NULL;
     sys->frame_last = &sys->frame_chain;
@@ -587,7 +599,12 @@ Flush(aout_stream_t *stream)
 
     vlc_tick_t unused;
     if (GetFrameTimestampLocked(stream, &sys->frames_flush_pos, &unused) != VLC_SUCCESS)
+    {
+        sys->frames_flush_pos = 0;
         msg_Warn(stream, "Flush: can't get paused position");
+    }
+
+    vlc_mutex_unlock(&sys->lock);
 
     if (RequestFlush(stream) != VLC_SUCCESS)
         return;



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/29e86638e6c14842e64539afb8b7d21573e60c27...a2a387856256f2e65030b79b295c94f2f55cadea

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/29e86638e6c14842e64539afb8b7d21573e60c27...a2a387856256f2e65030b79b295c94f2f55cadea
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance