896176 - [Music - MMS ] Not able to play AMR files(play from the sent item) from message view list

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Music, defect)

Description

[Leo]

1. Title: Not able to play AMR files(sent item) from message view list
2. Precondition: Have a couple of amr files in the device.
3. Tester's Action: 
- Compose a MMS message , attach amr files and send 
- Once the MMS message is recieved , try opening the sent file to play in the message view list.This is not 
4. Detailed Symptom (ENG.) :  When user tries to play the sent amr file in message view(play from the sent item) , it shows " unsupported format" and it doesn't play.
5. Expected: It should play the amr file.
6. Reproducibility: Y
1) Frequency Rate : 100%
7. Gaia Master/v1-train: Reproduced on v1-train
8. Gaia Revision: 3b2b0e422c3c772424d4b4e861986c81032d1d3f

Comment 1

[Leo]

Attachment: upload_files_bugzilla.rar

uploaded test files for reference

Comment 2

[Dominic Kuo [:dkuo]]

Here is the error log:

E/GeckoConsole(444): [JavaScript Warning: "Media resource blob:5c21b44a-7ec5-48c1-b290-dcfed73a99ec could not be decoded." {file: "app://music.gaiamobile.org/open.html" line: 0}]

Looks like gecko doesn't recognize amr files, so before music app startd to play it, gecko just return "could not be decoded". This is possible an issue of blob again, I don't think we can fix this in gaia, we need some help from gecko, Marco, can you help on this? thanks.

Comment 3

[Marco Chen [:mchen]]

(In reply to Dominic Kuo [:dkuo] from comment #2)
> gecko, Marco, can you help on this? thanks.

I found there is an additional log related to this issue.
"E/AMRExtractor(  574): illegal AMR frame type 12"
FT - 12 is "for future use" referred from spec.

But the strange thing is that playing the same file from media player directly didn't have this issue.

Hi Inder,

May I know any suggestion from you? Thanks.

Comment 4

[Inder]

Strange that it works fine in media player but not from message list.

For me, I don't see these files in music application and there aren't any errors in log either. Did you have to make any changes for it to recognize the file? Can you please upload a complete log.

Comment 5

[Marco Chen [:mchen]]

I found that this bug seems to be not a media bug but related to blob object.
Because
  1. The test file's size is 100912 bytes.
  2. When test file can be played, ARMExtractor will get 100912 from mDataSource->getSize().
  3. When test file can not be played, the size will be 166448.

So when AMRExtractor try to parse the during info, there will be a strange frame appeared on these additional bytes.

Comment 6

[Dominic Kuo [:dkuo]]

(In reply to Marco Chen [:mchen] from comment #5)
> I found that this bug seems to be not a media bug but related to blob object.
> Because
>   1. The test file's size is 100912 bytes.
>   2. When test file can be played, ARMExtractor will get 100912 from
> mDataSource->getSize().
>   3. When test file can not be played, the size will be 166448.
> 
> So when AMRExtractor try to parse the during info, there will be a strange
> frame appeared on these additional bytes.

This is similar to bug 850941, probably related?

Comment 7

[Marco Chen [:mchen]]

The current status is that

  1. The size returned by nsHTMLMeidaElement::mChannel->GetContentLength() is correct. (content process)
  2. In FileMediaResource::open(), it looked into gFileDataTable for getting FileDataInfo based on blob's URI.
    a. In this step the FileDataInfo::nsDOMFileFile::GetSize() returned the correct file size too. (content process)
  3. In NS_GetStreamForBlobURI, it tried to get input stream. (content process)
  4. nsDOMFileFile::GetInternalStream() get a input stream and the size returned by nsIFileInputStream->Available() is correct too. (parent process)
  5. FileMediaResource::GetLength() returned the wrong size from remote blob (content process)

-------------------------------------------------------------------------------

Hi Ben,

Could you give some suggestion here because the wrong file size will be reported by remote blob? Thanks.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Hm, can you attach the stack trace for where the value is incorrect in the child process?

Comment 9

[Marco Chen [:mchen]]

Hi Ben,

Thanks for your help again.

I found that the remote input stream in this case is nsMultiplexInputStream and it contained two stream. One for nsDOMFileFile and the other is nsPartialDOMFile on chrome process. The nsDOMFileFile will return the correct available size (100912) then plus the size from nsPartialDOMFile (65536). Finally the size will be a wrong one - 166448.

Could you give any hint that why there are two input streams for playing one file by Music Player?

Thanks.

Comment 10

[Marco Chen [:mchen]]

Additional info is that only the AMR file had this problem. I can't reproduce it by mp3 file.

Comment 11

[Marco Chen [:mchen]]

Hi Andrew,

Due to that I found this issue is not related to media playback but related to remote blobs (comment 7 and 9), do you know who is the expert on remote blob (Ben?)? And please give the help on this bug (I am not familiar with this area.) Thanks.

Comment 12

[Andrew Overholt [:overholt]]

Ben is the best person.  Can you attach a stack trace (see comment 8)?  An STR and gecko revision # and info about where this reproduces (b2g desktop or just device?).

This bug doesn't seem to be a blocker; what's the priority here?

Comment 13

[Marco Chen [:mchen]]

STR:
 Refer to bug description.

Gecko Revision:
 b2g18 (v1-train)

Where:
 I tested on device only.

Leo didn' set this as +, but I think it should be taken care by someone also.

Thanks for your team's help.

Comment 14

[Andrew Overholt [:overholt]]

> Leo didn' set this as +, but I think it should be taken care by someone also.

=> leo?

There are 2 AMR files in the attachment here (https://bugzilla.mozilla.org/attachment.cgi?id=778838).

Comment 15

[bhavana bajaj [:bajaj]]

Does this happen only on sending an AMR file in an MMS or any AMR file on the phone will not be played?

Comment 16

[Andrew Overholt [:overholt]]

Marco, please attach a stack trace.

Comment 17

[Marco Chen [:mchen]]

This back trace shows the points that MediaResource get wrong size as 166448 but it is combined by two input streams.

#0  nsMultiplexInputStream::Available (this=0x43d1fa40, _retval=0x420fccf8)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/xpcom/io/nsMultiplexInputStream.cpp:185
#1  0x40b15988 in Available (this=0x43d50c70, aAvailable=0x420fccf8)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/dom/ipc/Blob.cpp:269
#2  0x40c373aa in nsMultiplexInputStream::Available (this=0x43d1fa20, 
    _retval=0x420fcd30)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/xpcom/io/nsMultiplexInputStream.cpp:181
#3  0x40b14a0a in Available (this=<value optimized out>, _retval=0x420fcd30)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/dom/ipc/Blob.cpp:111
#4  0x40c373aa in nsMultiplexInputStream::Available (this=0x43d1f820, 
    _retval=0x420fcd70)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/xpcom/io/nsMultiplexInputStream.cpp:181
#5  0x408c6486 in FileMediaResource::EnsureSizeInitialized (this=0x43d50c10)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/content/media/MediaResource.cpp:1283
#6  0x408c65ac in FileMediaResource::GetLength (this=0x43d50c10)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/content/media/MediaResource.cpp:1192
#7  0x4089ba6c in nsMediaOmxReader::ReadMetadata (this=0x42b3fda0, 
    aInfo=0x420fce08, aTags=<value optimized out>)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/content/media/omx/nsMediaOmxReader.cpp:79
#8  0x408d115e in nsBuiltinDecoderStateMachine::DecodeMetadata (this=0x42c6acf0)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/content/media/nsBuiltinDecoderStateMachine.cpp:1825
#9  0x408d138c in nsBuiltinDecoderStateMachine::DecodeThreadRun (
    this=0x42c6acf0)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/content/media/nsBuiltinDecoderStateMachine.cpp:510
#10 0x4046249c in nsRunnableMethodImpl<void (nsPACMan::*)(), true>::Run (
    this=<value optimized out>) at ../../../dist/include/nsThreadUtils.h:366
#11 0x40c434aa in nsThread::ProcessNextEvent (this=0x43db2be0, 
    mayWait=<value optimized out>, result=0x420fceb7)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/xpcom/threads/nsThread.cpp:620
#12 0x40c237a6 in NS_ProcessNextEvent_P (thread=0x43d1f820, mayWait=true)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/objdir-gonk/xpcom/build/nsThreadUtils.cpp:237
#13 0x40c438f4 in nsThread::ThreadFunc (arg=<value optimized out>)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/xpcom/threads/nsThread.cpp:258
#14 0x415d99c8 in _pt_root (arg=<value optimized out>)
    at /home/marco/workspaces/B2G/mozilla-central/b2g18/nsprpub/pr/src/pthreads/ptthread.c:202
#15 0x400e9114 in __thread_entry (func=0x415d9969 <_pt_root>, arg=0x443929b0, 
    tls=<value optimized out>) at bionic/libc/bionic/pthread.c:217
#16 0x400e8c68 in pthread_create (thread_out=<value optimized out>, 
    attr=0x46252cfc, start_routine=0x415d9969 <_pt_root>, arg=0x443929b0)
    at bionic/libc/bionic/pthread.c:357

Comment 18

[Preeti Raghunath(:Preeti)]

Hi Andrew,

will this issue a cert blocker?

Comment 19

[Andrew Overholt [:overholt]]

Sorry, I forgot to respond here.  I doubt this is a cert blocker but it's a semi-broken new 1.1 feature so I agree with your leo+ decision.

Comment 20

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Ok, looking at this today.

Comment 21

[Preeti Raghunath(:Preeti)]

Bent,

Any update with this bug fix?

Comment 22

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I am so far unable to reproduce this bug, maybe just because I don't have a SIM that works in my phone. I've tried lots of different ways to simulate this problem and have yet to get any crashes.

Can someone who does experience this bug please try a build with the patch from bug 908432 (attachment 797323 [details] [diff] [review]) applied? It's possible that this bug could be fixed with that change.

Comment 23

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Also, it would help if someone could email me privately (bent at mozilla.com) with a zip of their sms database. I'd need the /data/local/indexedDB/chrome/226660312ssm.sqlite as well as the full contents of the /data/local/indexedDB/chrome/226660312ssm/ folder.

mchen, maybe yours?

Comment 24

[Marco Chen [:mchen]]

(In reply to ben turner [:bent] (needinfo? encouraged) from comment #22)
> I am so far unable to reproduce this bug, maybe just because I don't have a
> SIM that works in my phone. I've tried lots of different ways to simulate
> this problem and have yet to get any crashes.
> 

This bug is not a "crash" bug actually.

Comment 25

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Marco Chen [:mchen] from comment #24)
> This bug is not a "crash" bug actually.

Oops, right. I meant that I have not yet been able to reproduce the "unsupported format" error.

Comment 26

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Hm, somehow this needinfo got cleared. Marco, can you email me your sms database (and its supporting files)?

Comment 27

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I received an email from mchen with a database, though it was unfortunately empty. I'll leave the needinfo? set to see if mchen can find a different database.

In better news, though, he said this:

> And good news is this bug is gone after applying patch from bug 908432.

I think we should leo+ that bug ASAP.

Comment 28

[Marco Chen [:mchen]]

What steps I reproduced this bug are

  1. create a new MMS.
  2. select to attach from music
  3. choose AMR file and make sure it can play in this run.
  4. back to MMS editor.

  5. touch the attached file and choose view.
  6. music app will be lunched but can't play at this time.
  7. back to MMS editor.

Then I got the database for what Ben indicated.

Comment 29

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch for b2g18, v1

I finally tracked this down. We had a bug where slices were sometimes added as sub-blobs...

Marco, can you test with this patch please? I am pretty sure this is going to fix everything.

Comment 30

[Marco Chen [:mchen]]

Comment on attachment 801155 [details] [diff] [review]
Patch for b2g18, v1

Review of attachment 801155 [details] [diff] [review]:
-----------------------------------------------------------------

The feedback+ is not came from code review but from testing this patch on device.
And the testing result is positive.

Comment 31

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Thanks Marco!

Comment 32

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 801155 [details] [diff] [review]
Patch for b2g18, v1

Review of attachment 801155 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/9b95afb3cc6c

Ben, can you please make sure that your patches have the needed checkin metadata when setting checkin-needed. I'm not a big fan of guessing commit messages.

Comment 34

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Oops, sorry about that. Thanks!

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/9b95afb3cc6c