Closed
Bug 1378852
Opened 7 years ago
Closed 7 years ago
[Fennec][HLS] getBufferedPosition in GeckoHlsPlayer should always return a value >= 0.
Categories
(Firefox for Android Graveyard :: Audio/Video, enhancement)
Firefox for Android Graveyard
Audio/Video
Tracking
(firefox56 fixed)
RESOLVED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: kikuo, Assigned: kikuo)
References
Details
Attachments
(1 file)
ExoPlayer starts each playback with a base pts 60000000.
Occasionally, when seek to a position near to playback start, ExoPlayer may adjust itself to a final reset position which is less than PTS 60000000.
In that case, getBufferedPosition() will return a negative value, e.g. if ExoPlayer says the final reset position is 59500000 after a seek operation, we may get -500000 (59500000 - 60000000) for buffered position, and that will lead to an assertion |MOZ_ASSERT(aStart <= aEnd);| [1] while constructing a TimeInterval object [2].
According to the description for ExoPlayer API |getBufferedPosition| [3], UNKNOWN_TIME (a negative value, -1L) is returned when there's no estimated buffered position at that time, so we should return 0 in GeckoHlsPlayer::getBufferedPosition().
[1] http://searchfox.org/mozilla-central/rev/5e1e8d2f244bd8c210a578ff1f65c3b720efe34e/dom/media/Intervals.h#56
[2] http://searchfox.org/mozilla-central/rev/5e1e8d2f244bd8c210a578ff1f65c3b720efe34e/dom/media/hls/HLSDemuxer.cpp#647-648
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8884071 [details]
Bug 1378852 - Always return a value >= 0 for getBufferedPosition in GeckoHlsPlayer.
https://reviewboard.mozilla.org/r/155010/#review160126
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsPlayer.java:586
(Diff revision 1)
> @Override
> public long getBufferedPosition() {
> assertTrue(mPlayer != null);
> // Value returned by getBufferedPosition() is in milliseconds.
> - long bufferedPos = mPlayer.getBufferedPosition() * 1000;
> + long bufferedPos = Math.max(0, mPlayer.getBufferedPosition() * 1000);
> if (DEBUG) { Log.d(LOGTAG, "getBufferedPosition : " + bufferedPos + "(Us)"); }
You solved the intermittent crash issue!
To fulfill Math.max(long, long), please modify the code to 0L, mPlayer.getBufferdPosition() * 1000L.
And also amend the
http://searchfox.org/mozilla-central/rev/f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsPlayer.java#576
since the API has the same behavior as this one.
Thanks.
Attachment #8884071 -
Flags: review?(jacheng) → review+
Assignee | ||
Updated•7 years ago
|
Blocks: HLS_on_Fennec
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to James Cheng[:JamesCheng] from comment #3)
> Comment on attachment 8884071 [details]
> Bug 1378852 - Always return a value >= 0 for getBufferedPosition in
> GeckoHlsPlayer.
>
> https://reviewboard.mozilla.org/r/155010/#review160126
>
> :::
> mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/
> GeckoHlsPlayer.java:586
> (Diff revision 1)
> > @Override
> > public long getBufferedPosition() {
> > assertTrue(mPlayer != null);
> > // Value returned by getBufferedPosition() is in milliseconds.
> > - long bufferedPos = mPlayer.getBufferedPosition() * 1000;
> > + long bufferedPos = Math.max(0, mPlayer.getBufferedPosition() * 1000);
> > if (DEBUG) { Log.d(LOGTAG, "getBufferedPosition : " + bufferedPos + "(Us)"); }
>
> You solved the intermittent crash issue!
>
> To fulfill Math.max(long, long), please modify the code to 0L,
> mPlayer.getBufferdPosition() * 1000L.
>
> And also amend the
>
> http://searchfox.org/mozilla-central/rev/
> f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/mobile/android/geckoview/src/main/
> java/org/mozilla/gecko/media/GeckoHlsPlayer.java#576
>
> since the API has the same behavior as this one.
>
> Thanks.
Thanks for review.
Since the meaning of getDuration / getBufferedPosition are not exactly the same, I'll check the code flow, and handle it in Bug 1379056.
Comment hidden (mozreview-request) |
Pushed by kikuo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/52d810352774
Always return a value >= 0 for getBufferedPosition in GeckoHlsPlayer. r=JamesCheng
Comment 7•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•