Closed Bug 1069289 Opened 11 years ago Closed 11 years ago

Intermittent test_chaining.html | We should have received 2 metadataloaded event. http://mochi.test:8888/tests/content/media/test/variable-preskip.opus - got 1, expected 2

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
x86
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla35
Tracking Flags:
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
firefox-esr31 --- unaffected
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: cbook, Assigned: jwwang)

References

(
URL
)

Details

(Keywords: crash, intermittent-failure)

Attachments

(1 file)

1069289_take_audioendtime_when_update_playback_position_at_playback_end.patch
1.48 KB, patch
kinetik
: review+
lmandel
: approval-mozilla-beta+
Details | Diff | Splinter Review
Android 2.3 Emulator mozilla-inbound opt test mochitest-5 on 2014-09-18 03:35:02 PDT for push 9f35bdbd5054 slave: tst-linux64-spot-944 https://tbpl.mozilla.org/php/getParsedLog.php?id=48370381&tree=Mozilla-Inbound 209 INFO TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_chaining.html | We should have received 2 metadataloaded event. http://mochi.test:8888/tests/content/media/test/variable-preskip.opus - got 1, expected 2 212 INFO TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_chaining.html | The played region should be continuous.http://mochi.test:8888/tests/content/media/test/variable-preskip.opus - got 0, expected 1
It is possible for AudioSink to drain completely before we reach [1]. Since |mAudioCompleted| is true, we never get a chance to run AdvanceFrame() for audio-only files and playback position is not updated. Moreover, since the file is short, we switch from DECODER_STATE_DECODING to DECODER_STATE_COMPLETED immediately. We don't get a chance to run AdvanceFrame() to update playback position in DECODER_STATE_DECODING either. By the time we get to [2], |mEndTime| is -1 for variable-preskip.opus, and |clockTime| will be 0 in the end. UpdatePlaybackPosition(0) doesn't give us a chance to dispatch pending metadata and we fail the test which expects 2 metadataloaded events. Fix - We should take |mAudioEndTime| into account when updating playback position at the end of playback at [2]. [1] http://hg.mozilla.org/mozilla-central/file/25a98bac9264/content/media/MediaDecoderStateMachine.cpp#l2453 [2] http://hg.mozilla.org/mozilla-central/file/25a98bac9264/content/media/MediaDecoderStateMachine.cpp#l2482 Try for Android 2.3: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9005a1edfa5c&searchQuery=Android%202.3%20Emulator%20try%20opt%20test%20mochitest-5 Try for all platforms: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9005a1edfa5c The oranges on B2G are either about graphics or related to media recorder which is a known issue.
Assignee: nobody → jwwang
Status: NEW → ASSIGNED
Attachment #8501485 - Flags: review?(kinetik)
Attachment #8501485 - Flags: review?(kinetik) → review+
Keywords: checkin-needed
Blocks: 1052735
https://hg.mozilla.org/mozilla-central/rev/60c38a8155ce
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
AFAICT, these failures only ever happened on trunk, but it appears that the underlying defect goes back further. Please nominate this for Aurora uplift unless I'm way off base here :)
status-firefox33: --- → wontfix
status-firefox34: --- → affected
status-firefox35: --- → fixed
status-firefox-esr31: --- → unaffected
Flags: needinfo?(jwwang)
Comment on attachment 8501485 [details] [diff] [review] 1069289_take_audioendtime_when_update_playback_position_at_playback_end.patch Approval Request Comment [Feature/regressing bug #]:unknown [User impact if declined]:fix test failure of test_chaining.html in bug 1069289. [Describe test coverage new/current, TBPL]: Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1de5da477f3f Overall looks fine. The oranges are already there before this patch. [Risks and why]: Low. The change is simple and has been tested on Central for a while. [String/UUID change made/needed]:none
Attachment #8501485 - Flags: approval-mozilla-aurora?
Flags: needinfo?(jwwang)
Comment on attachment 8501485 [details] [diff] [review] 1069289_take_audioendtime_when_update_playback_position_at_playback_end.patch 34 merged to beta today.
Attachment #8501485 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8501485 [details] [diff] [review] 1069289_take_audioendtime_when_update_playback_position_at_playback_end.patch Beta+
Attachment #8501485 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: