783927 - Audio cuts out shortly after playing MP4 on Android

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Aaron Train [:aaronmt]]

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]]

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)]

Attachment: 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]]

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)]

Attachment: Handle zero length reads to fix audio

Address review comments, carry forward r+.

Comment 5

[cajbir (:cajbir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/94174af1fd88

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/94174af1fd88