[music] duration of mp3 is not correct

RESOLVED FIXED

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: sotaro, Unassigned)

Tracking

({regression})

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+, b2g-v1.2 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

6 years ago
+++ This bug was initially created as a clone of Bug #909243 +++

The bug is created based on Bug 909243 Comment 12.

g_128orig.mp3 should have duration of 30 seconds only but it will be show as 41 seconds. So when player is playing at 30 seconds then player will end to play.

I confirmed the symptom on today's v1.2 and master hamachi.
Reporter

Updated

6 years ago
blocking-b2g: --- → koi?
Reporter

Updated

6 years ago
Summary: [Nexus 4][Android 4.3][music] unable to play the song (.mp3) via music app → [music] duration of mp3 is not correct
Reporter

Comment 1

6 years ago
By applying the patch, the duration of g_128orig.mp3 became correct. mozilla::MP3FrameParser seems not work correctly.
Reporter

Comment 2

6 years ago
Most recent master seems to fix the problem.
 - gecko: e55ebbb92007b838189d067ca398a8f81c5a1436

But the problem is still present on v1.2. From it, Bug 922334 seems to fix the problem. attachment 812945 [details] [diff] [review] in Bug 922334 seems to fix the problem. It blocks mp3 file parsing before AudioTrack is created in OmxDecoder::TryLoad().

Within TryLoad(), ProcessCachedData(0, true) is called. TryLoad() and OmxDecoder::NotifyDataArrived() code might request mozilla::MP3FrameParser to count mp3's length as overlapping.
Reporter

Comment 3

6 years ago
Even when applied attachment 812945 [details] [diff] [review] to v1.2, the problem was not fixed. The patch seems not related to this bug.
(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> Within TryLoad(), ProcessCachedData(0, true) is called. TryLoad() and
> OmxDecoder::NotifyDataArrived() code might request mozilla::MP3FrameParser
> to count mp3's length as overlapping.

But this should not happen. The MP3 frame parser should keep track of the offset up to which it has parsed the file. There have been some major changes to the parser recently in  bug 910996. You might try to revert attachment https://bugzilla.mozilla.org/attachment.cgi?id=799915 to go back to the old version.

Comment 5

6 years ago
Is this happening on all mp3 files or something specific to one file.
Flags: needinfo?(sotaro.ikeda.g)
Reporter

Comment 6

6 years ago
(In reply to Hema Koka [:hema] from comment #5)
> Is this happening on all mp3 files or something specific to one file.

Is the above could be checked by using various mp3 files?
Flags: needinfo?(sotaro.ikeda.g)
Keywords: qawanted
Some of the MP3's I've had bad durations reported on include:

http://people.mozilla.org/~cpearce/vbr.mp3   (duration 2:00, *still* reported as 34s by MP3FrameParser).
http://nxt-level.com/music/02.%20everlast%20-%20black%20jesus.mp3
MP3s from http://folkradio.ru/nagrani/
http://nxt-level.com/music/DJ%20Shadow%20feat.%20Mos%20Def%20-%20Six%20Days.mp3

There are others too.
Reporter

Comment 8

6 years ago
Confirmed a patch in Bug 919572 fixes the problem.
Depends on: 919572
Keywords: qawanted
Here are my findings:

Testing on m-c gecko, the issue is not replicable.  g_128orig.mp3 from test_media/music in Comment 0 shows duration as 30 sec and play for 30 secs 
Build Id:20131015040202
OS Version 1.3.0.0-prerelease
Gecko mozilla-central : c079fe98d21fc2c24dbf333d1341855c9280f3d0
Gaia master: f5c2e987fcc859e371fedb44006ca94fc0ebcb28

On gecko v1.2 the issue is replicable,  g_128orig.mp3 from test_media/music  shows duration as 41 sec and play for 30 secs 
Build Id:20131015040202
OS Version 1.2.0.0-prerelease
Gecko mozilla-aurora : 292baf182d8778772335fd2f6fa84eb82e31b906
Gaia master: f5c2e987fcc859e371fedb44006ca94fc0ebcb28 

Testing mp3 files in Comment 7, shows correct duration on both v1.2 and v1.3. 

This is a gecko issue, as confirmed by Sotaro in Comment 8 and should be fixed by landing mozilla-aurora fix of bug 919572 in v1.2
Reporter

Updated

6 years ago
Blocks: 927884
Since we have the fix on master, it will be fixed in 1.3

not blocking this for 1.2
blocking-b2g: koi? → 1.3+
(In reply to Hema Koka [:hema] from comment #10)
> Since we have the fix on master, it will be fixed in 1.3
> 
> not blocking this for 1.2

Confused. The bug Sotaro references that fixes this issue affects 1.2. And the above discussion talks about this reproducing on 1.2. And this is likely a regression as well.

Can you clarify?
Flags: needinfo?(hkoka)
Jason,

We are able to reproduce this in 1.2. However, it does not affect the audio output of the mp3. The issue is only on the duration being displayed and the movement of the slider to the end once the 30 sec mark is passed. Mp3 plays correctly. Given that input, we did not mark it a blocker during the bug traige meeting. 

However, I just tested this on my Leo which has an old version of 1.1 and the duration is displayed correctly for the test mp3 file g_128orig.mp3 -- so this probably is a regression issue. Can you please confirm. We can then re-evaluate if we need to mark this a blocker.
Flags: needinfo?(jsmith)
Sure. QA Wanted - Can someone check if this reproduces on 1.1?
Flags: needinfo?(jsmith)
Keywords: qawanted

Updated

6 years ago
QA Contact: sarsenyev
Flags: needinfo?(hkoka)

Comment 14

6 years ago
The bugs doesn't reproduce on Leo 1.1 MOZ RIL,
MP3 files show correct duration time

Device: Leo 1.1 MOZ RIL
BuildID: 20131018041205
Gaia: 39b0203fa9809052c8c4d4332fef03bbaf0426fc
Gecko: 7fedb6a967ea
Version: 18.0
Firmware Version: US_20130912
Keywords: qawanted
That confirms this is a regression. And based on cpearce's comments, there's multiple mp3 files affected. A music player isn't exactly all that useful if it's displaying the playback duration incorrectly. Renom.
blocking-b2g: 1.3+ → koi?
Keywords: regression
marking this a blocker - regression issue
blocking-b2g: koi? → koi+
When Bug fix: https://bugzilla.mozilla.org/show_bug.cgi?id=919572 lands on 1.2, this fix can be verified and and closed
Hema, since the dependent bug 919572 has been landed...do we just need to verify and close this bug?
Reporter

Updated

6 years ago
Keywords: qawanted
yes, that's correct (in reply to Candice's comment 18)

Thanks Sotaro for adding qawanted flag.
Can be closed, seems to work fine on 10/28 Buri aurora build 
- music plays exactly the time as it's showing for the file duration (tested with MP3 files from comment 7)

Gaia   2ef9bc3c7a6de228b63e6ba3613eb0c0dd639c59
SourceStamp 4a94d2ea9d37
BuildID 20131028004002
Version 26.0a2
Keywords: qawanted
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.