Android video playback suffers from bug 669556

RESOLVED FIXED in mozilla11

Status

()

Core
Audio/Video
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

(Blocks: 1 bug)

Trunk
mozilla11
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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.
(Assignee)

Comment 2

6 years ago
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
(Assignee)

Comment 3

6 years ago
Changing summary.
Summary: More Android audio fixes → Android video playback suffers from bug 669556
(Assignee)

Updated

6 years ago
Blocks: 679071
(Assignee)

Comment 4

6 years ago
Created attachment 576020 [details] [diff] [review]
content/media patch v0

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)
(Assignee)

Comment 5

6 years ago
Created attachment 576023 [details] [diff] [review]
sydneyaudio patch v0

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+
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
No longer depends on: 695612

Comment 6

6 years ago
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+
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/22f2cc4d1a04
http://hg.mozilla.org/integration/mozilla-inbound/rev/f4381030a5ab
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/22f2cc4d1a04
https://hg.mozilla.org/mozilla-central/rev/f4381030a5ab
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Depends on: 695612
You need to log in before you can comment on or make changes to this bug.