992854 - [tarako] No sound when play some specific m4a music

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Peipei Cheng (needinfo if you need my action)]

Attachment: 0102_88khz_75kbps.m4a

Steps to reproduce
------------------------------------------------------
1. Download attached music
2. Put music to Device
3. Launch Music and select the music to play
   --> There is no sound coming out.


Test Build
------------------------------------------------------
Gaia      fbf194db193f12b0db070c87bc99f6bb42e28dc0
Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/5ff1d2a9fbc1
BuildID   20140404164001
Version   28.1
ro.build.version.incremental=eng.cltbld.20140404.201608
ro.build.date=Fri Apr  4 20:16:14 EDT 2014 


Buri works fine.

Comment 1

[Peipei Cheng (needinfo if you need my action)]

Attachment: adb logcat

attached adb logcat. Test starts: 04-07 10:39:15.420

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Looking at the log, maybe it has to do with the sample rate?  I think we might need to take a look at a variety of sample rates if this is the case...

10:39:29.800 I/SPRDAACDecoder(   87): sampleRate : 44100
... 
04-07 10:39:29.830 I/SPRDAACDecoder(   87): aac sampleRate is 44100, but now is 88200

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I think we need to figure out if it is the sample rate and what sample rates we support currently.  qawanted.

Comment 4

[Ming]

i tacked the codes and logs.

It's in gecko/media/libcubeb/cubeb_opensl.c, in function opensl_stream_init, code is :
obviously the opensl limits the rate between 8k-48kHZ  //
  if (stream_params.rate < 8000 || stream_params.rate > 48000 ||
      stream_params.channels < 1 || stream_params.channels > 32 ||
      latency < 1 || latency > 2000) {
    return CUBEB_ERROR_INVALID_FORMAT;
  }

Comment 5

[Ming]

I just changed the rate limit to 8k-88.2k , it worked fine.  


Is it better to use audiotrack just like android?

Comment 6

[Bruce Sun [:brsun]]

Hi Matthew,
Currently we limit the sampling rate between 8k~48kHz in opensl_stream_init(). Do we have any concern on enlarging the range of min/max limit?

Comment 7

[Matthew Gregan [:kinetik]]

I don't know of any, AFAIK it's just a sanity check but it seems redundant since we have one in the higher-level cubeb_stream_init now (which uses the wider limit of 1..192kHz).

Comment 8

[Jason Smith [:jsmith]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #3)
> I think we need to figure out if it is the sample rate and what sample rates
> we support currently.  qawanted.

I don't think I'd send that over to QA Wanted actually - I'd ask a media dev for that info.

However, I think we should keep the qawanted request to determine if this reproduces on 1.3.

Comment 9

[Ming]

(In reply to Jason Smith [:jsmith] from comment #8)
Hi jason, have u got any reply from media dev?

Comment 10

[Jason Smith [:jsmith]]

Not yet.

Comment 11

[Ming]

(In reply to Jason Smith [:jsmith] from comment #10)
> Not yet.

Thanks. Pls help to make this issue clear in time. 
We need it.

Comment 12

[James Zhang (Spreadtrum)]

(In reply to Ming from comment #4)
> i tacked the codes and logs.
> 
> It's in gecko/media/libcubeb/cubeb_opensl.c, in function opensl_stream_init,
> code is :
> obviously the opensl limits the rate between 8k-48kHZ  //
>   if (stream_params.rate < 8000 || stream_params.rate > 48000 ||
>       stream_params.channels < 1 || stream_params.channels > 32 ||
>       latency < 1 || latency > 2000) {
>     return CUBEB_ERROR_INVALID_FORMAT;
>   }
Wayen, please comment here.

Comment 13

[James Zhang (Spreadtrum)]

Attachment: bug255119_support_88200_rate.patch

wayen, please review this patch.

Comment 14

[Wayne Chen [:xwaynec]]

Comment on attachment 8404367 [details] [diff] [review]
bug255119_support_88200_rate.patch

Hi Matthew,

Could you help to review this patch?
Thanks

Sincerely,
Wayne

Comment 15

[Star Cheng [:scheng]]

(In reply to James Zhang from comment #13)
> Created attachment 8404367 [details] [diff] [review]
> bug255119_support_88200_rate.patch
> 
> wayen, please review this patch.


Just a note:
AudioStream keep one second audio buffer to ensure play smoothly. The sample rate is higher, the buffer size is larger. http://dxr.mozilla.org/mozilla-central/source/content/media/AudioStream.cpp#426

In this kind of memory constricted device (eg. tarako), there is more probability to encounter OOM phenomenon.

Comment 16

[Matthew Gregan [:kinetik]]

Comment on attachment 8404367 [details] [diff] [review]
bug255119_support_88200_rate.patch

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

I'd rather just remove the invalid format check completely and let the higher-level cubeb_stream_init handle it, but this is simple so I'm fine with taking it as is.

Comment 17

[Joe Cheng [:jcheng] (please needinfo)]

triage: m4a should be supported. 1.3T+

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/2d6b24e35aef

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2d6b24e35aef

Comment 20

[James Zhang (Spreadtrum)]

Need land the patch on v1.3t.

Comment 21

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/9937dbb49b69

Comment 22

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Fixed on todays 1.3T build.

1.3T Environmental Variables:
Device: Tarako 1.3T MOZ
BuildID: 20140428014001
Gaia: 8895b180ed636069473703d0e7b73086989601ce
Gecko: 7caf4b5abfce
Version: 28.1
Firmware Version: sp6821

Comment 23

[[:fabrice] Fabrice Desré]

Comment on attachment 8404367 [details] [diff] [review]
bug255119_support_88200_rate.patch

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

James, I guess you need that for Dolphin?

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/e69f1b6d5422

Comment 25

[James Zhang (Spreadtrum)]

(In reply to Fabrice Desré [:fabrice] from comment #23)
> Comment on attachment 8404367 [details] [diff] [review]
> bug255119_support_88200_rate.patch
> 
> Review of attachment 8404367 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> James, I guess you need that for Dolphin?

Thank you.

Comment 26

[Ming]

The limit now is in libopenSLES [frameworks/wilhelm/src/android/AudioPlayer_to_android.cpp:android_audioPlayer_checkSourceSink].

I will change the limit to 8k-88.2khz.