Last Comment Bug 675839 - Linux libsydneyaudio backend updates playback position infrequently
: Linux libsydneyaudio backend updates playback position infrequently
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: mozilla8
Assigned To: cajbir (:cajbir)
:
Mentors:
Depends on:
Blocks: 573161 589595 591790 610930 610931 620721 623470 648595 666092 666271
  Show dependency treegraph
 
Reported: 2011-08-01 19:04 PDT by cajbir (:cajbir)
Modified: 2011-08-07 16:33 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (1.85 KB, patch)
2011-08-01 19:06 PDT, cajbir (:cajbir)
kinetik: review-
Details | Diff | Splinter Review
Fix (1.89 KB, patch)
2011-08-01 23:02 PDT, cajbir (:cajbir)
kinetik: review+
Details | Diff | Splinter Review

Description cajbir (:cajbir) 2011-08-01 19:04:48 PDT
The libsydneyaudio backend for linux writes a largish chunk of audio at a time in a block call to Alsa. This is followed by an update of the internal number of bytes read. The large granularity of the write and update causes issues when media playback attempts to read the playback position to find the current time.

An example of this is in bug 648595 when an end fragment is specified for media. If a check is made to see if the media has reached an end time just before the end time is reached, the next check can have the audio time quite a bit in advance of the desired end time due to the large granularity of the playback position increments.
Comment 1 cajbir (:cajbir) 2011-08-01 19:06:57 PDT
Created attachment 549996 [details] [diff] [review]
Fix

Perform the blocking write in a loop in smaller increments, updating the playback position after each successful small write.
Comment 2 Matthew Gregan [:kinetik] 2011-08-01 22:04:35 PDT
Comment on attachment 549996 [details] [diff] [review]
Fix

Review of attachment 549996 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libsydneyaudio/src/sydney_audio_alsa.c
@@ +252,4 @@
>    while(nframes>0) {
>      if (s->resumed) {
>        avail = snd_pcm_avail_update(s->output_unit);
> +      avail = avail < 64 ? avail : 64;

This undoes the workaround added in bug 573924.

@@ +267,5 @@
>          return SA_ERROR_SYSTEM;
>        }
>      } else {
>        nframes -= frames;
> +      size_t bytes = snd_pcm_frames_to_bytes(s->output_unit, frames);

Declare |bytes| at the top of a block to avoid compiler warnings.
Comment 3 cajbir (:cajbir) 2011-08-01 23:02:44 PDT
Created attachment 550013 [details] [diff] [review]
Fix

Address review comments.
Comment 5 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-08-03 02:28:04 PDT
http://hg.mozilla.org/mozilla-central/rev/512bcdfdb77f
Comment 6 Chris Pearce (:cpearce) 2011-08-07 16:26:46 PDT
Looks like this actually worked around the bug we were hitting in alsa causing audio-writes to not return, causing timeouts on Tinderbox. There have been no such intermittent timeouts on Linux boxes on Tinderbox on the branches with this changeset landed. This then fixes (at least) these random orange bugs:

[Bug 573161] Intermittent timeout in test_progress.html, test_reactivate.html, test_replay_metadata.html, and test_seek.html on Linux
[Bug 591790] Frequent timeouts in test_buffered.html, test_bug465498.html, test_bug495145.html, test_framebuffer.html, followed by a "4 test timeouts, giving up."
[Bug 610931] Intermittent test_bug495145.html | Test timed out followed by shutdown timeout
[Bug 620721] Intermittent test_timeupdate_small_files.html | Test timed out. followed by shutdown timeout
[Bug 666092] Frequent timeouts in test_replay_metadata.html, test_seek.html, test_seek_out_of_range.html and test_timeupdate_small_files.html followed by a "4 test timeouts, giving up"
[Bug 666271] Frequent timeouts in test_bug495145.html, test_framebuffer.html, test_load_source.html and test_new_audio.html followed by a "4 test timeouts, giving up"

This significantly drops the media orange count. Way to go doublec!

Note You need to log in before you can comment on or make changes to this bug.