Android remote decoder doesn't return video frames on Android 4.2.2

VERIFIED FIXED in Firefox 54

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
Last year

People

(Reporter: jhlin, Assigned: jhlin)

Tracking

({regression})

Trunk
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54+ fixed, firefox55 fixed)

Details

Attachments

(1 attachment)

The size in the buffer info returned by 4.2.2 video decoder is 0 and the buffer will be dropped in OnOutput.
Comment on attachment 8865772 [details]
Bug 1363276 - discard video output buffers according to presentation time rather than size.

https://reviewboard.mozilla.org/r/137396/#review140590

What about the audio decoder, can this happen there, too?
Is it even save to decode with zero-sized buffers, isn't that a bug in the codec implementation?

::: dom/media/platforms/android/RemoteDataDecoder.cpp:135
(Diff revision 1)
>        if (!mDecoder->mInputInfos.Find(presentationTimeUs, inputInfo)
>            && !isEOS) {
>          return;
>        }
>  
> -      if (size > 0) {
> +      if (presentationTimeUs >= 0) {

size is now unused and can be removed.
Attachment #8865772 - Flags: review?(esawin) → review+
Comment on attachment 8865772 [details]
Bug 1363276 - discard video output buffers according to presentation time rather than size.

https://reviewboard.mozilla.org/r/137396/#review140590

s/save/safe
Comment on attachment 8865772 [details]
Bug 1363276 - discard video output buffers according to presentation time rather than size.

https://reviewboard.mozilla.org/r/137396/#review140590

This only applies to video for it uses surface/graphic buffer as output target.
ACodec fixed it after 4.2.2: https://android.googlesource.com/platform/frameworks/av/+/21ad778dcfcddb8f8fd9dc3fe4992fbef246c511
Assignee: nobody → jolin
Priority: -- → P1
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0eb6bcebaf3d
discard video output buffers according to presentation time rather than size. r=esawin
https://hg.mozilla.org/mozilla-central/rev/0eb6bcebaf3d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Ioana,
Can you help test it with the latest nightly?
Flags: qe-verify+
Flags: needinfo?(ioana.chiorean)
Track 54+ as OOP is enabled in 54.
Tested by both latest nightly and beta.  Using Google Nexus4 by Android 4.2.2

STR: 
1. open browser
2. navigate to youtube
3. choose random video, play, pause, stop, reload for several times.
4. tap another video by recommended list.
5. repeat step 3
6. test other video source, like facebook.

EXPECTED RESULT:
- video can be played, paused, reloaded successfully.

ACTUAL RESULT:
beta
- none of youtube video can be played; only placeholder(spinning icon) was showed.
- facebook video can't be played.
nightly
- complete STR can be performed successfully.
Comment on attachment 8865772 [details]
Bug 1363276 - discard video output buffers according to presentation time rather than size.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1257777
[User impact if declined]: video fails to play on Android 4.2.2 devices
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: yes. see comment 12
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: presentation time is more reliable than size
[String changes made/needed]: none
Attachment #8865772 - Flags: approval-mozilla-beta?
Comment on attachment 8865772 [details]
Bug 1363276 - discard video output buffers according to presentation time rather than size.

https://reviewboard.mozilla.org/r/137396/#review140590

> size is now unused and can be removed.

it is also possible for presentationTimeUs to be negative... Should set a particular value (like INT64_MIN) rather than dropping all frames with a negative timestamp.
Will open another bug to handle negative timestamp frames.
Bug 1366706 was filed as follow-up of comment 14.
Comment on attachment 8865772 [details]
Bug 1363276 - discard video output buffers according to presentation time rather than size.

This fix a critical issue that video fails to play on Android 4.2.2 devices and was verified. Beta54+. Should be in 54 beta 10.
Attachment #8865772 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Prestigio PSP5508DUO Android 4.4.2

STR: 
1. open browser
2. navigate to youtube,facebook, tiff.ro etc.
3. choose random video, play, pause, stop, reload for several times.

Expected and Actual Results:
- media process is showed separately
- media is played correctly
Status: RESOLVED → VERIFIED
Flags: needinfo?(ioana.chiorean)
Remove qe-verify based on comment 19.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.