Closed Bug 783927 Opened 7 years ago Closed 7 years ago

Audio cuts out shortly after playing MP4 on Android

Categories

(Core :: Audio/Video, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: aaronmt, Assigned: cajbir)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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)
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
Summary: Audio cuts out shortly after playing MP4 → Audio cuts out shortly after playing MP4 on Android
Version: unspecified → Trunk
QA Contact: chris.double
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.
Assignee: nobody → chris.double
Status: NEW → ASSIGNED
Attachment #653639 - Flags: review?(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.
Attachment #653639 - Flags: review?(cpeterson) → review+
Address review comments, carry forward r+.
Attachment #653639 - Attachment is obsolete: true
Attachment #653652 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/94174af1fd88
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.