Closed Bug 1552607 Opened 5 months ago Closed 4 months ago

seeking intermittently failed on Android

Categories

(Firefox for Android :: Audio/Video, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 69
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 69+ fixed
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: alwu, Assigned: jhlin)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Form from bug1548446 comment17, there are two different seeking issue on Android, which would cause intermittent failures.

This bug is used to investigate why seeking would fail on Android.

Priority: -- → P2

In bug 1540748/D28167 the demuxed sample timestamp was used to judge
whether audio decoder returns invalid output timestamp and reports
error for them. Unfortunately, Android may return buffers prior to
flush() and cause false alarm.

To make sure only requested buffers are reported, keep track of input
intervals and discard those that are outside the intervals rather
than compare with demux timestamp. Also log out-of-range buffers to
help debugging.

ProcessOutput() for buffers prior to Flush() could be scheduled to run
after the flush promise is resolved because MediaCodec buffer callback
runs asynchronously in both remote decoder process and content process.
To help check the validness of buffer, a session ID increased by flush
is added to both RemoteDataDecoder and remote codec service and will be
passed through IPC. If the passed ID doesn't agree with current session
ID, it means the buffer doesn't belong to current session and should be
discard.

Depends on D36381

Attachment #9070101 - Attachment is obsolete: true
Attachment #9074875 - Attachment description: Bug 1552607 - p2: check sample session ID when processing audio output. r?jya → Bug 1552607 - p2: check sample session ID when processing buffers. r?jya
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6385b8616d54
p1: filter out invalid input buffers. r=jya
https://hg.mozilla.org/integration/autoland/rev/53a39e402451
p2: check sample session ID when processing buffers. r=jya

Backed out 2 changesets for causing checkstyle failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/3de1b396b7177de25726848b8f62872eeb6072c8

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=53a39e4024511c051187db2a6c501c56c23a77ea&searchStr=checkstyle&selectedJob=254341852

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=254341852&repo=autoland&lineNumber=3045

[task 2019-07-01T23:59:25.147Z] 23:59:25 INFO - TEST-UNEXPECTED-FAIL | android-checkstyle | Checkstyle rule violations were found. See the report at: https://queue.taskcluster.net/v1/task/cju4VesKR2aF7nM7NCHpiA/runs/0/artifacts/public/android/checkstyle
[task 2019-07-01T23:59:25.147Z] 23:59:25 INFO - TEST-UNEXPECTED-FAIL | /builds/worker/workspace/build/src/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/Sample.java
[task 2019-07-01T23:59:25.148Z] 23:59:25 INFO - TEST-UNEXPECTED-FAIL | <error column="75" line="76" message="Parameter session should be final." severity="error" source="com.puppycrawl.tools.checkstyle.checks.FinalParametersCheck" />

Flags: needinfo?(jolin)
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f6b0141f7b1f
p1: filter out invalid input buffers. r=jya
https://hg.mozilla.org/integration/autoland/rev/f2d9515c3843
p2: check sample session ID when processing buffers. r=jya
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69

John, should we uplift this seeking fix to Fennec ESR 68? Can this bug affect real users or just our automated tests (like bug 1548446)?

There will be no Fennec 69 release. Fennec users will be switched to the ESR 68 channel for the next 6-12 months, so the only way to release a fix to Fennec users will be to uplift to ESR.

(In reply to Chris Peterson [:cpeterson] from comment #8)

John, should we uplift this seeking fix to Fennec ESR 68? Can this bug affect real users or just our automated tests (like bug 1548446)?

There will be no Fennec 69 release. Fennec users will be switched to the ESR 68 channel for the next 6-12 months, so the only way to release a fix to Fennec users will be to uplift to ESR.

It could affect real users. I made a mistake here and caused bug 1563734, so I'll request uplift for both after the later is landed and verified in central.

Flags: needinfo?(jolin)

There will be no Fennec 69 release. Fennec users will be switched to the ESR 68 channel for the next 6-12 months, so the only way to release a fix to Fennec users will be to uplift to ESR.

It could affect real users. I made a mistake here and caused bug 1563734, so I'll request uplift for both after the later is landed and verified in central.

Thanks! I'll update the bugs' status flags so to track for Fennec ESR 68 uplift.

[Tracking Requested - why for this release]:

John says this video seeking bug could affect real Fennec users and we should uplift the fix to Fennec ESR 68.0.x or 68.1. We will also want to uplift the fix for bug 1563734, which was a regression from this bug.(In reply to John Lin [:jhlin][:jolin] from comment #9)

Regressed by: 1563734
No longer regressed by: 1563734
Regressions: 1563734

Comment on attachment 9074874 [details]
Bug 1552607 - p1: filter out invalid input buffers. r?jya

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: The patch of bug 1540748 included in 68 causes a seeking failure regression that could be annoying to users. Please note that bug 1563734 fixed a regression by this patch and should also be uplifted.
  • User impact if declined: Random MP4 video playback errors after seeking.
  • Fix Landed on Version: 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patch eliminates a racing issue.
  • String or UUID changes made by this patch:
Attachment #9074874 - Flags: approval-mozilla-esr68?
Attachment #9074875 - Flags: approval-mozilla-esr68?

Comment on attachment 9074874 [details]
Bug 1552607 - p1: filter out invalid input buffers. r?jya

Fixes a new regression in 68 causing problems when seeking. Approved for Fennec 68.1b3.

Attachment #9074874 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attachment #9074875 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
You need to log in before you can comment on or make changes to this bug.