Closed
Bug 1118632
Opened 9 years ago
Closed 9 years ago
MediaCodecReader: duration is wrong after replay.
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: bechen, Assigned: bechen)
References
Details
Attachments
(1 file, 1 obsolete file)
2.58 KB,
patch
|
bechen
:
review+
|
Details | Diff | Splinter Review |
STR: 1. Enable MediaCodec preference 2. Open a http streaming by browser 3. At the end of stream, press play button to trigger replay The duration is wrong after replay. It looks like the mp3 Incremental parser is triggered, we should not trigger the parser all the time. == call stack mozilla::MediaDecoderStateMachine::SetDuration (this=this@entry=0xb1a0e800, aDuration=69741178) at /home/benjamin/Documents/hg/mozilla-central/dom/media/MediaDecoderStateMachine.cpp:1358 #1 0xb5aa0454 in mozilla::MediaDecoderStateMachine::UpdateEstimatedDuration (this=0xb1a0e800, aDuration=<optimized out>) at /home/benjamin/Documents/hg/mozilla-central/dom/media/MediaDecoderStateMachine.cpp:1380 #2 0xb5af7746 in mozilla::MediaCodecReader::ParseDataSegment (this=this@entry=0xb1a0e400, aBuffer=aBuffer@entry=0xb1cdd008 "[3\357\340\024ԗ\005\214\215\202\002\023\002\023\232\231Ö\241ʧ3\"L\237Q\225\301\265\210ċ\\\253\341B\325ܪ\310y\026+\317+\276\363\326\004\245F\304\324\366\025\356\257Lۗ\267=\315\370\222{\260\364\266\233M\232\345\320\001ሔ\345\020N\331`\304eJ\031\202\210\022", aLength=aLength@entry=2896, aOffset=aOffset@entry=697589) at /home/benjamin/Documents/hg/mozilla-central/dom/media/omx/MediaCodecReader.cpp:696 #3 0xb5af77e4 in mozilla::MediaCodecReader::NotifyDataArrived (this=0xb1a0e400, aBuffer=0xb1cdd008 "[3\357\340\024ԗ\005\214\215\202\002\023\002\023\232\231Ö\241ʧ3\"L\237Q\225\301\265\210ċ\\\253\341B\325ܪ\310y\026+\317+\276\363\326\004\245F\304\324\366\025\356\257Lۗ\267=\315\370\222{\260\364\266\233M\232\345\320\001ሔ\345\020N\331`\304eJ\031\202\210\022", aLength=<optimized out>, aOffset=697589) at /home/benjamin/Documents/hg/mozilla-central/dom/media/omx/MediaCodecReader.cpp:581 #4 0xb5a8eda8 in mozilla::MediaDecoderStateMachine::NotifyDataArrived (this=0xb1a0e800, aBuffer=<optimized out>, aLength=<optimized out>, aOffset=aOffset@entry=697589) at /home/benjamin/Documents/hg/mozilla-central/dom/media/MediaDecoderStateMachine.cpp:1562 #5 0xb5a8ee84 in mozilla::MediaDecoder::NotifyDataArrived (this=0xb1c330c0, aBuffer=<optimized out>, aLength=<optimized out>, aOffset=697589) at /home/benjamin/Documents/hg/mozilla-central/dom/media/MediaDecoder.cpp:1498
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8548033 -
Flags: review?(cajbir.bugzilla)
Attachment #8548033 -
Flags: feedback?(brsun)
Comment 2•9 years ago
|
||
Comment on attachment 8548033 [details] [diff] [review] bug-1118632.v01.patch Review of attachment 8548033 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for catching this. Basically it seems good to me. Incrementally parsing definitely should be skipped if we don't need it. Please help also move |mMP3FrameParser = new MP3FrameParser(...)| a little bit lower to make sure |mMP3FrameParser| is allocated only while parsing a so-called MP3 file. ::: dom/media/omx/MediaCodecReader.cpp @@ +689,5 @@ > return NS_ERROR_FAILURE; > } > > + bool isMP3 = mDecoder->GetResource()->GetContentType().EqualsASCII(AUDIO_MP3); > + if (isMP3) { It would be good to have the name of this boolean variable like incrementalParserNeeded or something, so that these two |if| statements can be combined into one intuitively.
Attachment #8548033 -
Flags: feedback?(brsun) → feedback+
Updated•9 years ago
|
Attachment #8548033 -
Flags: review?(cajbir.bugzilla) → review+
Assignee | ||
Comment 3•9 years ago
|
||
r=cajbir try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=496e9389c3a7
Attachment #8548033 -
Attachment is obsolete: true
Attachment #8550242 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 4•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/684267d1eaf4
Assignee: nobody → bechen
Keywords: checkin-needed
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/684267d1eaf4
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•