Last Comment Bug 783927 - Audio cuts out shortly after playing MP4 on Android
: Audio cuts out shortly after playing MP4 on Android
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla17
Assigned To: cajbir (:cajbir)
: cajbir (:cajbir)
Mentors:
http://people.mozilla.com/~atrain/mob...
Depends on:
Blocks: 759945
  Show dependency treegraph
 
Reported: 2012-08-19 18:44 PDT by Aaron Train [:aaronmt]
Modified: 2012-08-21 06:27 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Handle zero length reads to fix audio (4.00 KB, patch)
2012-08-20 20:23 PDT, cajbir (:cajbir)
cpeterson: review+
Details | Diff | Splinter Review
Handle zero length reads to fix audio (4.36 KB, patch)
2012-08-20 22:01 PDT, cajbir (:cajbir)
cajbir.bugzilla: review+
Details | Diff | Splinter Review

Description Aaron Train [:aaronmt] 2012-08-19 18:44:11 PDT
Run attached test-case:


Perhaps there's a core audio dependency yet to land for Android; if so, please invalidate this bug.

--
20120819171203
http://hg.mozilla.org/integration/mozilla-inbound/rev/c676b554c7bb
Samsung Galaxy Nexus (Android 4.1.1)
Comment 1 Aaron Train [:aaronmt] 2012-08-20 06:26:10 PDT
See bug URL: 

adb shell am start -a android.intent.aciton.VIEW -n org.mozilla.fennec/.App -d http://people.mozilla.com/~atrain/mobile/tests/test.mp4

I/OmxPlugin( 2022): width: 320 height: 240 component: OMX.google.h264.decoder format: 19 stride: 320 sliceHeight: 240 rotation: 0
I/SoftAAC2( 2022): Reconfiguring decoder: 44100 Hz, 2 channels
I/OmxPlugin( 2022): channelCount: 2 sampleRate: 44100
Comment 2 cajbir (:cajbir) 2012-08-20 20:23:20 PDT
Created attachment 653639 [details] [diff] [review]
Handle zero length reads to fix audio

Stagefright sometimes returns zero length reads and we were treating this as end of stream. This patch ignores zero byte buffers allowing audio to continue playing.
Comment 3 Chris Peterson [:cpeterson] 2012-08-20 21:10:30 PDT
Comment on attachment 653639 [details] [diff] [review]
Handle zero length reads to fix audio

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

LGTM with some nits.

::: content/media/plugins/nsMediaPluginReader.cpp
@@ +255,5 @@
>      return false;
>    }
>    mAudioSeekTimeUs = -1;
>  
> +  // Ignore empty buffers which stagefright media read will sproadically return

s/sproadically/sporadically/

@@ +265,5 @@
>  
>    PRUint32 frames = frame.mSize / (2 * frame.mAudioChannels);
>    CheckedInt64 duration = FramesToUsecs(frames, frame.mAudioSampleRate);
>    if (!duration.isValid()) {
>      return NS_ERROR_FAILURE;

DecodeAudioData()'s return type is bool, but we are returning NS_ERROR_FAILURE (0x80004005) here. We should probably return false or change DecodeAudioData() to return an nsresult like most other nsMediaPluginReader functions.

::: media/omx-plugin/OmxPlugin.cpp
@@ -532,5 @@
> -        mVideoBuffer->data(), 
> -        mVideoBuffer->size(),
> -        mVideoBuffer->range_offset(),
> -        mVideoBuffer->range_length(),
> -        unreadable);

I had a patch to `#ifdef DEBUG` this noisy log message, but it looks like you got it first. :)

@@ +588,5 @@
>      else
>        return ReadAudio(aFrame, aSeekTimeUs);
>    }
>    else if (err == ERROR_END_OF_STREAM)
>      return false;

This `err == ERROR_END_OF_STREAM` check can be removed because this case is covered by the `err == OK` check below.

@@ +590,5 @@
>    }
>    else if (err == ERROR_END_OF_STREAM)
>      return false;
> +
> +  return err == OK; 

Remove trailing whitespace.
Comment 4 cajbir (:cajbir) 2012-08-20 22:01:53 PDT
Created attachment 653652 [details] [diff] [review]
Handle zero length reads to fix audio

Address review comments, carry forward r+.
Comment 6 Ed Morley [:emorley] 2012-08-21 06:27:32 PDT
https://hg.mozilla.org/mozilla-central/rev/94174af1fd88

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