Closed Bug 1382557 Opened 7 years ago Closed 7 years ago

Doesn't show the correct duration for media

Categories

(Core :: Audio/Video: Playback, defect, P3)

Other Branch
defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: alwu, Assigned: jwwang)

Details

Attachments

(1 file)

Tested on OSX with FF54 and Nightly56.0a1. 

STR.
1. go to https://media-testing.updog.co/using.mp4

Expected.
2. video should show the correct duration

Actual.
2. video doesn't show the total duration of video
Assignee: nobody → ayang
Ths duration is ok when plays locally.
It's regression because I could see the duration before, but I've already forgotten what's the version I used. (maybe 53 or 52)
DurationChanged() is called before demuxer parsing in network case.
Assignee: ayang → nobody
Flags: needinfo?(jwwang)
(In reply to Alastor Wu [:alwu][please needinfo? me][7/26-8/3 PTO] from comment #2)
> It's regression because I could see the duration before, but I've already
> forgotten what's the version I used. (maybe 53 or 52)

Yeah, it reminds me bug 1370177. :-)
Issue can also see on this URL, it seems very easy to be reproduced.

https://media-testing.updog.co/Toe.mp3
Summary: Doesn't show the correct duration for mp4 → Doesn't show the correct duration for media
Assignee: nobody → jwwang
Flags: needinfo?(jwwang)
Priority: -- → P3
Attachment #8890241 - Flags: review?(cpearce)
Comment on attachment 8890241 [details]
Bug 1382557 - the duration should be finite when we can get one from the metadata.

https://reviewboard.mozilla.org/r/161364/#review167130

::: dom/media/MediaDecoder.cpp:768
(Diff revision 1)
>    Invalidate();
>  
>    EnsureTelemetryReported();
> +
> +  // The duration is no longer infinite when we get one from the metadata.
> +  if (mInfo->mMetadataDuration && IsInfinite()) {

We could also have IsInfinite() check whether we've got a duration from the metadata, rather than relying on manually setting the state here.

I'll let you make the decision as to which approach to use here.
Attachment #8890241 - Flags: review?(cpearce) → review+
Comment on attachment 8890241 [details]
Bug 1382557 - the duration should be finite when we can get one from the metadata.

https://reviewboard.mozilla.org/r/161364/#review167130

> We could also have IsInfinite() check whether we've got a duration from the metadata, rather than relying on manually setting the state here.
> 
> I'll let you make the decision as to which approach to use here.

SGTM. I will take your approach. Thanks!
Comment on attachment 8890241 [details]
Bug 1382557 - the duration should be finite when we can get one from the metadata.

https://reviewboard.mozilla.org/r/161364/#review167130

> SGTM. I will take your approach. Thanks!

It turns out that the duration is not updated unless you invoke DurationChanged() manually in MetadataLoaded(). This is because DurationChanged() is triggered by mStateMachineDuration (state-mirroring) before MetadataLoaded() is called. So I have to stick to the original approach though.
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/52ea7b299388
the duration should be finite when we can get one from the metadata. r=cpearce
https://hg.mozilla.org/mozilla-central/rev/52ea7b299388
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
QA Whiteboard: [good first verify]
Successfully Reproduced the bug in Nightly 56.0a1 (2017-07-20) on windows 10 x64

Verified as fixed with latest Firefox Beta 56.0b2 (Build ID: 20170810180547) 

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [good first verify] → [good first verify] [bugday-20170809]
I have reproduced this bug with Nightly 56.0a1 (2017-07-20) on ubuntu 16.04 LTS!

This bug's fix is verified with latest Beta!


Build  ID  : 20170818155657 
User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0

[bugday-20170816]
You need to log in before you can comment on or make changes to this bug.