1375411 - [Android] cubeb doesn't call drain when playing specific file

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P1)

Description

[Alastor Wu [:alwu]]

Fork from bug1374956 comment4, the audio would never end when playing the "very-short.mp3".

This issue can be reproduced on Nexus5 with Android 6.0 and Z3C with Android 5.1.

---

You have two reproduce ways,

Method-A : 
Run mochitest dom/media/test/test_playback_rate.html, and then you would see the timeout.

Method-B : 
STR.
1. goto the page https://people-mozilla.org/~alwu/playbackRate.html
2. change the playback rate of "very-short.mp3" (arbitrary rate, but not 1) 
3. play the audio

Expect.
4. audio end

Actual.
4. audio doesn't end

Note : the page has another normal audio, but it won't have this symptom. It can end successfully even you change its playback rate.

Comment 1

[Alastor Wu [:alwu]]

Hi, Paul,
Could you help me take a look about this issue?
Thanks!

Comment 2

[Alex Chronopoulos [:achronop]]

I will take this one since I have done the latest work in OpenSL ES backend. I will try to repro later today. It's been a while from last time I built for Android.

Comment 3

[Paul Adenot (:padenot)]

JW should know about this. Any idea?

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

If this issue happens only after changing playback rate, you can add some logs to AudioStream::GetTimeStretched() to see if there is anything abnormal.

Comment 5

[Alex Chronopoulos [:achronop]]

In case you have a build ready could you capture logs with cubeb flag on (cubeb:5)? I see you have already logs (without cubeb though) in Bug 1374956#c4.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Log with Cubeb_OpenSL

Here is the log.
It seems that the cubeb doesn't finish draining?

Comment 7

[Alastor Wu [:alwu]]

Attachment: Log with Cubeb_OpenSL (Normal playback rate, successful case)

This is the case that the media can end successfully. 
In this case, we don't change its playback rate (rate=1).

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Alastor Wu [:alwu][please needinfo? me] from comment #6)
> Created attachment 8881467 [details]
> Log with Cubeb_OpenSL
> 
> Here is the log.
> It seems that the cubeb doesn't finish draining?

When draining starts, cubeb_opensl calls SetMarkerPosition() and wait for the callback at [2] to ensure all data are consumed by the player before notifying DRAINED. It looks like there is something in the calculation of |written_duration| such that the player will never reach the position.

[1] http://searchfox.org/mozilla-central/rev/fdb811340ac4e6b93703d72ee99217f3b1d250ac/media/libcubeb/src/cubeb_opensl.c#326-335
[2] http://searchfox.org/mozilla-central/rev/fdb811340ac4e6b93703d72ee99217f3b1d250ac/media/libcubeb/src/cubeb_opensl.c#219-227

Comment 9

[Alex Chronopoulos [:achronop]]

The problem with non working log is that it does not render any frame. It's the first callback that returns 0 frames and goes to drain (which fails to do). It does not make sense that the callback return 0 frame from the first iteration. If you see the working case log, callback is called several times returning 240 frames before it goes to drain. Do you have any clue why 0 frames are returned in the first callback?

Comment 10

[Alastor Wu [:alwu]]

Now I saw is that the |mTimeStretcher| doesn't get any data even we have already called |mTimeStretcher::putSamples()|, the |mTimeStretcher->numSamples()| would always be ZERO [1].

Actually, I've seen that we indeed put something into FIFOSampleBuffer, I'm guessing whether we put the data into the wrong buffer or something else...

Now I'm looking into the implementation in soundtouch, but if you have any idea about this symptom, please feel free to leave any comment.

Thanks!

[1] https://goo.gl/wpQ85R

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

The input/output of TimeStretcher is not a linear function. E.g. you might need to feed 2000 samples before you can get 500 output samples. That's why we have a while loop to keep feeding data until we have enough output samples.

Comment 12

[Alastor Wu [:alwu]]

(In reply to JW Wang [:jwwang] [:jw_wang] from comment #11)
> The input/output of TimeStretcher is not a linear function. E.g. you might
> need to feed 2000 samples before you can get 500 output samples. That's why
> we have a while loop to keep feeding data until we have enough output
> samples.

In this case, we can't get any output samples and would leave the loop when we get the chunk without containing any frame [1].
Any idea?

[1] https://goo.gl/DCPrZv

Comment 13

[JW Wang [:jwwang] [:jw_wang]]

If there is no data in the audio queue, it will just return an empty chunk.

http://searchfox.org/mozilla-central/rev/5bb6dfcb3355a912c20383578bc435c1d4ecf575/dom/media/mediasink/AudioSink.cpp#262-265

Comment 14

[Alastor Wu [:alwu]]

I was busy on the bug1274919 last week, I'll back to this bug soon.

Comment 15

[Alastor Wu [:alwu]]

Because people.org has been closed, the new testing URL is https://goo.gl/tXqMmf.

Comment 16

[Alastor Wu [:alwu]]

Attachment: Bug 1375411 - notify drained if we didn't write any sample.

The time stretching would fail on very short audio file.

Since the input/output of time-stretcher is not a linear function, it only returns
result if we feed it enough samples.

When the resample returns zero sample in this situation, cubeb should call drained
initiative.

Review commit: https://reviewboard.mozilla.org/r/156472/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/156472/

Comment 17

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f75831e15cc45a1632a850f77b750006ecb6cd7

Comment 18

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2c718ebd1e182132495c855049c3ce0b33e223c

Comment 19

[Alex Chronopoulos [:achronop]]

Please note you have to deliver the patch on cubeb repo: https://github.com/kinetiknz/cubeb

Comment 20

[Alastor Wu [:alwu]]

(In reply to Alex Chronopoulos [:achronop] from comment #19)
> Please note you have to deliver the patch on cubeb repo:
> https://github.com/kinetiknz/cubeb

Thanks! I've created the pull-request in cubeb repo [1].

[1] https://github.com/kinetiknz/cubeb/pull/339

Comment 21

[Alastor Wu [:alwu]]

Close this bug as fixed because the changed has been merged into cubeb repo [1].

[1] https://github.com/kinetiknz/cubeb/pull/339

Comment 22

[Alastor Wu [:alwu]]

Hi, Alex,
How often we update our code from git cubeb repo?
Just want to know when my changing would be applied on m-c, because I would like to see how many test cases failure can be fixed by this changing.
Thanks!

Comment 23

[Alex Chronopoulos [:achronop]]

I am in the middle of another cubeb import in Bug 1380233. As soon as this one is landed I will start the new one.

Comment 24

[Alastor Wu [:alwu]]

(In reply to Alex Chronopoulos [:achronop] from comment #23)
> I am in the middle of another cubeb import in Bug 1380233. As soon as this
> one is landed I will start the new one.

Thank you!

Comment 25

[Alastor Wu [:alwu]]

Sorry for another NI,
Could you add me to cc list when next time you file bug for updating the cubeb?
Thanks!

Comment 26

[Alastor Wu [:alwu]]

Leave this bug open until the change is merged into m-c.

Comment 27

[Alex Chronopoulos [:achronop]]

Bug 1381015, I added you in the list.

Comment 28

[Alastor Wu [:alwu]]

Code merged in bug1381015.