Closed Bug 695986 Opened 8 years ago Closed 8 years ago

Android video playback suffers from bug 669556

Categories

(Core :: Audio/Video, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: kinetik, Assigned: kinetik)

References

Details

Attachments

(2 files)

I claimed that the testcase in bug 693905 worked once the  patches I mentioned had been applied, but that was incorrect.  There's an additional fix required that I had experimented with while debugging and was unknowingly still testing against due to doing partial source rebuilds.

The necessary fix is to force an initial position update via AudioParent::Notify when the first write occurs, otherwise the position estimator doesn't start returning valid times until the timer fires, which happens one second after initializing the AudioParent.
Hrm, there's another problem where it'll stutter when it begins playing, or within the first few seconds, and then never recover.  If you seek to a new position, it plays fine.  That sound suspiciously like bug 669556, which will still exist here due to reintroducing the Wait().  Maybe I'll need to fix bug 695612 before this is going to work properly.
I've verifying via logging that the remaining problem is simply bug 669556.  The AudioParent timer changes merely alter the behaviour (but not in a consistently good way).  It still makes sense to only run the timer when the stream is active, though.  I'll use this bug to track that fix and some other general improvements.

Fixing this correctly involves fixing bug 695612.
Depends on: 695612
Changing summary.
Summary: More Android audio fixes → Android video playback suffers from bug 669556
Blocks: 679071
Fix bug 669556 on Android by removing the broken code.  We can do this now that Fennec is single process in the NativeUI world order, since the bug affecting the remoted audio code (bug 695612) no longer matters.
Attachment #576020 - Flags: review?(chris)
Only start the AudioTrack playing once it signals its buffers are full.  Remove the unnecessary fake-blocking-write sleep in sa_stream_write.  Allocate a small Java byte array during init to be used for write calls--this avoids churning the GC by allocating on every write.
Attachment #576023 - Flags: review?(doug.turner)
Attachment #576020 - Flags: review?(chris) → review+
Status: NEW → ASSIGNED
No longer depends on: 695612
Comment on attachment 576023 [details] [diff] [review]
sydneyaudio patch v0

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

::: media/libsydneyaudio/src/sydney_audio_android.c
@@ +277,4 @@
>    (*jenv)->PopLocalFrame(jenv, NULL);
>  
> +  ALOG("%p - New stream %u %u bsz=%u min=%u obsz=%u", s,  s->rate, s->channels,
> +       s->bufferSize, minsz, s->output_buf_size);

nit: remove extra space after the 's,'.  Also, the second line should be aligned under the %
Attachment #576023 - Flags: review?(doug.turner) → review+
Depends on: 695612
You need to log in before you can comment on or make changes to this bug.