1052206 - [MADAI][Multimedia] Attached mp3 file is occurred tick noise.

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jaemin Song]

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36

Steps to reproduce:

1. Copy attached mp3 file to SD card and insert Sdcard to device.
2. Execute music player and play attached mp3 file.
3. When start playback, please listen carefully sound.
4. You can hear tick noise.

I'll attach mp3 file.
Attached mp3 file is FireFoxOS ring tone file.(Es Loop)



Actual results:

As a result of PCM data analysis in audio stream,
I have found silence sound in 0~1 sec.
So, player is occurred tick noise.

As a result of debugging,
When start playback, MediaDecdeorStateMachine calls "PlaySilence" function.
So, silence PCM data is added.

It will be checked why player occurs "PlaySilence" function.
Please confirm this problem.

Thanks.

Comment 1

[Jaemin Song]

[Blocking Requested - why for this release]:

Comment 2

[Jaemin Song]

Attachment: es-loop.mp3

I have attached mp3 file.

Comment 3

[Dietrich Ayala (:dietrich)]

QAWANTED for branch checks.

Eric, can your team look into why the problem occurs with this particular file?

Comment 4

[Joshua Mitchell (Inactive)]

Jaemin - I'd like to just get a little more information so I can fully understand 
"4. You can hear tick noise."

When I think of 'tick noise' I think of the noise a stopwatch makes - a tick,tick,tick noise. Listening to the audio file I do not hear anything like this.  
I do hear what I might call a 'crackle' or 'audio pop' DURING the first note of the song / ring tone as you might hear from a low-quality audio file. 

So from my understanding the noise you are hearing (is it only 1 tick, or multiple ticks) is right at the beginning before the audio starts? For me the audio starts almost right away.

I'm testing on a Flame device so if my understanding of what you are hearing is accurate then this simply means a no-repro on Flame devices.

Comment 5

[Joshua Mitchell (Inactive)]

Issue does NOT repro on Flame 2.1, Flame 2.0, Flame 1.4 and Buri 2.1

Actual Results: On the flame device only a small audio distortion / pop during the first note - on the Buri no sound distortion is heard at all.


Device: Flame Master
Build ID: 20140812132618
Gaia: 9f35fca9d818b26c06aa6b7e5c0bef25886f8f20
Gecko: 7fc96293ada8
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.0
Build ID: 20140813070951
Gaia: e215fbf0cb16063b3d2f3e6a4e588c3550b6becb
Gecko: 5490354af7dc
Version: 32.0 (2.0)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 1.4
Build ID: 20140811081112
Gaia: d47fecdcaa4aefb754561114edd22fb23a92b98d
Gecko: fbee340d0830
Version: 30.0 (1.4)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Device: Buri Master
Build ID: 20140810234109
Gaia: 19ed3c9e78eaf234cc08484bde6998ae21552ba5
Gecko: a9b43778f0c2
Version: 34.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 6

[Jaemin Song]

(In reply to Joshua Mitchell [:Joshua_M] from comment #4)
> Jaemin - I'd like to just get a little more information so I can fully
> understand 
> "4. You can hear tick noise."
> 
> When I think of 'tick noise' I think of the noise a stopwatch makes - a
> tick,tick,tick noise. Listening to the audio file I do not hear anything
> like this.  
> I do hear what I might call a 'crackle' or 'audio pop' DURING the first note
> of the song / ring tone as you might hear from a low-quality audio file. 
> 
> So from my understanding the noise you are hearing (is it only 1 tick, or
> multiple ticks) is right at the beginning before the audio starts? For me
> the audio starts almost right away.
> 
> I'm testing on a Flame device so if my understanding of what you are hearing
> is accurate then this simply means a no-repro on Flame devices.

My word(tick noise) selection was wrong.
I had heard 'crackle' after starting audio playback.(Between 0~1 seconds)
To compare normal PCM and AudioStream(gecko) PCM, I'll attach PCM data.
You can find that "silence" PCM data is added between 0~1 sec.
("silence" PCM data is added from "MediaDecdeorStateMachine::PlaySilence" function.)
Because of this "silence" PCM, problem is occurred.
Please refer attached data.

Thanks.

Comment 7

[Jaemin Song]

Attachment: es-loop_PCM_DATA.zip

To compare PCM data, I have attached PCM data.

Comment 8

[Jaemin Song]

Attachment: Compare_PCMdata.jpg

PCM comparison image.

Comment 9

[Eric Chou [:ericchou] [:echou]]

(In reply to Dietrich Ayala (:dietrich) from comment #3)
> QAWANTED for branch checks.
> 
> Eric, can your team look into why the problem occurs with this particular
> file?

ok, we can take a look.

Comment 10

[Eric Chou [:ericchou] [:echou]]

Star is digging into this.

Comment 11

[Jason Smith [:jsmith]]

With the new information the reporter provided, can we try to reproduce this again?

Comment 12

[Joshua Mitchell (Inactive)]

based on the verification from the reporter that a 'crackle' sound is the type of distortion heard

This bug DOES reproduce on Flame 2.1, Flame 2.0, Flame 1.4

Actual Results - a crackle/pop/distortion sound is heard during the first note (sec 0-1)

Device: Flame Master
Build ID: 20140812132618
Gaia: 9f35fca9d818b26c06aa6b7e5c0bef25886f8f20
Gecko: 7fc96293ada8
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.0
Build ID: 20140813070951
Gaia: e215fbf0cb16063b3d2f3e6a4e588c3550b6becb
Gecko: 5490354af7dc
Version: 32.0 (2.0)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 1.4
Build ID: 20140811081112
Gaia: d47fecdcaa4aefb754561114edd22fb23a92b98d
Gecko: fbee340d0830
Version: 30.0 (1.4)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

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

This issue does NOT repro on Buri 2.1

Actual Results - NO sound distortion is heard during playback 

Device: Buri Master
Build ID: 20140810234109
Gaia: 19ed3c9e78eaf234cc08484bde6998ae21552ba5
Gecko: a9b43778f0c2
Version: 34.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 13

[Joshua Mitchell (Inactive)]

also:

This issue does NOT reproduce on 2.1 Open-C

Actual Results - no crackle on the 1st note

Device: Open_C Master
Build ID: 20140814005107
Gaia: 5e074831f9ddacdf6f622a6dffaecb626f740be8
Gecko: 5299864050ee
Version: 34.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 14

[Star Cheng [:scheng]]

I observed the playedFrames and sampleTime is not mapping[1] when the phenomenon (audio crackle) occurs. It causes to invoke playsilence() function which mentioned in commnet 0. Because the playedFrames and sampleTime are decided by MP3 decoder, there is something wrong in google mp3 decoder. I try to use QC MP3 decoder instead of OMX.google.mp3.decoder, and I can *not* duplicate the phenomenon (audio crackle).

Hamachi - QC Mp3 decoder       [can *not* duplicate]
Flame - OMX.google.mp3.decoder [can duplicate]
OpenC - OMX.google.mp3.decoder [can duplicate] (As I saw the log and hear the sound of play music, the audio crackle phenomenon occurs in openC)

[1] I/AudioSink( 1207): [star] playedFrames 623 sampleTime 1151 missingFrames 528
    I/AudioSink( 1207): [star] PlaySilence

Hi, Michael

Can you help to explain why CAF use google mp3 decoder instead of QC mp3 decoder for OpenC/Flame (8610 platform)?

Comment 15

[Michael Vines [:m1] [:evilmachines]]

Duplicate of bug 1053186 perhaps?

mp3 playback should be using audio offload in v2.0, the Flame/OpenC images from comment 14 and lower are surely JB-based so aren't close enough approximations of what we'd expect to see in a full v2.0 device image.  Obtaining the unmodified Flame KK vendor image and trying to reproduce the issue there, with bug 1053186 landed, could help as a next step.

If in-depth support is then needed under Gecko please raise an SR.

Comment 16

[Jason Smith [:jsmith]]

QA Wanted to test this on 2.0 with a kitkat Flame image

Comment 17

[vasanth]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #15)
> Duplicate of bug 1053186 perhaps?

Reporter mentioned its not same. This is reproducible after disabling audio offload mode as well.

Comment 18

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

This issue does reproduce on the latest 2.0 build of the v165 kk base.  There is a small crackle heard right as the file starts.

Environmental Variables:
Device: Flame 2.0
BuildID: 20140819030000
Gaia: 287a2c725a5c14e5dc1d48e3158ffc79c7d1ea33
Gecko: 6329352ca531b977979451e77e5862af485388b2
Version: 32.0 (2.0) 
Firmware Version: v165
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 19

[Star Cheng [:scheng]]

According to comment 17 and comment 18, audio crackle still occurs in kk-based + v2.0 and that's different from  bug 1053186. We need CAF's support to check why the mp3 playback decoder is google instead of QC.

Comment 20

[Jaemin Song]

(In reply to Star Cheng [:scheng] from comment #19)
> According to comment 17 and comment 18, audio crackle still occurs in
> kk-based + v2.0 and that's different from  bug 1053186. We need CAF's
> support to check why the mp3 playback decoder is google instead of QC.

If mp3 duration is above 60 sec, 
player is decoding using QC's offload for mp3 file.
But if duration is less than 60 sec,
player is decoding using Google's decoder.

Comment 21

[Jason Smith [:jsmith]]

QA Wanted to see if this reproduces with the fix included from bug 1053186.

Comment 22

[Jaemin Song]

(In reply to Jason Smith [:jsmith] from comment #21)
> QA Wanted to see if this reproduces with the fix included from bug 1053186.

After applying bug 1053186 patch, this issue is occurred.
Because this issue is different with bug 1053186, issue is reproduced.
(please refer comment 17)
bug 1053186 issue is occurred when using offload.
But this issue is occurred when using google's decoder.

Thanks.

Comment 23

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

This issue still occurs on today's 2.0 Flame build with the v165 base.  A faint crackle can be heard at the start of the file.

Environmental Variables:
Device: Flame 2.0 (319 MB)
Build ID: 20140821030000
Gaia: ed257456c512c3b345f5a89e9211581e7ae0f1c0
Gecko: 6329352ca531b977979451e77e5862af485388b2
Version: 32.0 (2.0)
Firmware Version: 165
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 24

[bhavana bajaj [:bajaj]]

(In reply to Star Cheng [:scheng] from comment #19)
> According to comment 17 and comment 18, audio crackle still occurs in
> kk-based + v2.0 and that's different from  bug 1053186. We need CAF's
> support to check why the mp3 playback decoder is google instead of QC.

Ni :mvines for input here.

Comment 25

[Michael Vines [:m1] [:evilmachines]]

SR please

Comment 26

[Sotaro Ikeda [:sotaro]]

(In reply to Star Cheng [:scheng] from comment #19)
> According to comment 17 and comment 18, audio crackle still occurs in
> kk-based + v2.0 and that's different from  bug 1053186. We need CAF's
> support to check why the mp3 playback decoder is google instead of QC.

QC decoder is hw decoder, even when QC decoder is supported, if the decoder is already in use, google one is used in this case. It seems that this bug hit google decoder's defect. If it is correct, it seems also necessary to address.

Comment 27

[Jaemin Song]

(In reply to Sotaro Ikeda [:sotaro] from comment #26)
> QC decoder is hw decoder, even when QC decoder is supported, if the decoder
> is already in use, google one is used in this case. It seems that this bug
> hit google decoder's defect. If it is correct, it seems also necessary to
> address.

I had also confirmed dump file in Google MP3 decoder.
But, I had not found silent data in MP3 decoder's dump file.
MP3 decoder's dump file was normal.
Silent sound data is added by "MediaDecoderStateMachine::PlaySilence" function.

Thanks.

Comment 28

[bhavana bajaj [:bajaj]]

Jaemin, I think there needs to be an SR opened on CAF side, so please follow-up there, i am removing the nom for now.

Comment 29

[Jaemin Song]

(In reply to Sotaro Ikeda [:sotaro] from comment #26)
> QC decoder is hw decoder, even when QC decoder is supported, if the decoder
> is already in use, google one is used in this case. It seems that this bug
> hit google decoder's defect. If it is correct, it seems also necessary to
> address.
(In reply to bhavana bajaj [:bajaj] from comment #28)
> Jaemin, I think there needs to be an SR opened on CAF side, so please
> follow-up there, i am removing the nom for now.

Dear Sotaro,

This issue is not google decoder's defect.

To confirm reason why player uses google decoder for mp3,
Please refer comment 20.
And I had also checked MP3 decoder.
Please refer comment 27.

Could you check this issue again?

Thanks.

Comment 30

[Sotaro Ikeda [:sotaro]]

(In reply to Jaemin Song from comment #27)
> 
> I had also confirmed dump file in Google MP3 decoder.
> But, I had not found silent data in MP3 decoder's dump file.
> MP3 decoder's dump file was normal.
> Silent sound data is added by "MediaDecoderStateMachine::PlaySilence"
> function.

Jaemin, do you know why "MediaDecoderStateMachine::PlaySilence" add silence?

Comment 31

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #30)
> Jaemin, do you know why "MediaDecoderStateMachine::PlaySilence" add silence?

One possibility is timestamp is not correct.

Comment 32

[Sotaro Ikeda [:sotaro]]

Other possibility is decoding is too slow.

Comment 33

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #30)
> Jaemin, do you know why "MediaDecoderStateMachine::PlaySilence" add silence?

Jaemin, can you also provide a logcat log when the problem happen?

Comment 34

[Jaemin Song]

Dear Sotaro,

I don't know why "MediaDecoderStateMachine::PlaySilence" is called.
I have found that below code calls "PlaySilence".
[MediaDecoderStateMachine::AudioLoop()]
if (missingFrames.value() > 0) {
      // The next audio chunk begins some time after the end of the last chunk
      // we pushed to the audio hardware. We must push silence into the audio
      // hardware so that the next audio chunk begins playback at the correct
      // time.

I have checked time stamp and decoder speed in log file.
As a result of confirm,
I could not find problem in time stamp and decoder speed.
I'll attach log file.
Please confirm log file.

Thanks.

Comment 35

[Jaemin Song]

Attachment: Log_0905.txt

I have attached log file.
You can find "playing 849 frames of silence".

Thanks

Comment 36

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Star, Sotaro, could you help to check the logs on Comment#35 to help to move this bug forward?

Thanks

Vance

Comment 37

[Star Cheng [:scheng]]

(In reply to Sotaro Ikeda [:sotaro] from comment #26)
> (In reply to Star Cheng [:scheng] from comment #19)
> > According to comment 17 and comment 18, audio crackle still occurs in
> > kk-based + v2.0 and that's different from  bug 1053186. We need CAF's
> > support to check why the mp3 playback decoder is google instead of QC.
> 
> QC decoder is hw decoder, even when QC decoder is supported, if the decoder
> is already in use, google one is used in this case. It seems that this bug
> hit google decoder's defect. If it is correct, it seems also necessary to
> address.

That's a good viewpoint. I check the related code according to Sotaro's concern, and find QC mp3 (audio/mpeg) decoder is sw decoder. And QC force translate hw codec to sw codec[1]. But the priority of google sw mp3 decoder is more than QC's one[2]. So we can raise the priority of QC's mp3 decoder, the issue should be resolved. 

We can figure out the google decoder's defect as well. Of course, it will spend more time to find it.


[1]  sp<MediaSource> OMXCodec::Create(...)

    if ((!strcmp(mime, "audio/mpeg"))
            && createEncoder == false
            && flags == kHardwareCodecsOnly) {
        flags = kSoftwareCodecsOnly;
        ALOGE("Forcing SoftwareCodec for mime type %s\n", mime);
    }

[2] /system/etc/media_codecs.xml
        <!-- Audio Software  -->
        <MediaCodec name="OMX.google.mp3.decoder" type="audio/mpeg" />
        <MediaCodec name="MP3Decoder" type="audio/mpeg" />

Comment 38

[Star Cheng [:scheng]]

(In reply to Jaemin Song from comment #34)
> Dear Sotaro,
> 
> I don't know why "MediaDecoderStateMachine::PlaySilence" is called.

You can see the comment 31 and comment 32. That cases will cause the function be invoked.

> I have found that below code calls "PlaySilence".
> [MediaDecoderStateMachine::AudioLoop()]
> if (missingFrames.value() > 0) {
>       // The next audio chunk begins some time after the end of the last
> chunk
>       // we pushed to the audio hardware. We must push silence into the audio
>       // hardware so that the next audio chunk begins playback at the correct
>       // time.
> 
> I have checked time stamp and decoder speed in log file.
> As a result of confirm,
> I could not find problem in time stamp and decoder speed.
> I'll attach log file.
> Please confirm log file.
> 
> Thanks.

Comment 39

[Sotaro Ikeda [:sotaro]]

I am going to investigate about why PlaySilence() happens.

Comment 40

[Sotaro Ikeda [:sotaro]]

OMXCodec::read() outputs the following times of frames. But the first two frames does not have valid frame data. Then audio playback starts from the third frame(52244 us). And each frame's duration is 26122 us. It requests "frame size = 4608". But the third frame's size only have 1212 byte. Therefore, PlaySilence() is added.

- 0 us //frame size = 0
- 26122 us // frame size = 0
- 52244 us // frame size = 1212
- 78367 us // frame size = 4608
- 104489 us // frame size = 4608
- 130612 us // frame size = 4608
- 156734 us // frame size = 4608
- 182857 us // frame size = 4608

Comment 41

[Sotaro Ikeda [:sotaro]]

On android's stagefright, audio playback is very simple. Just push audio data to AudioTrack. It does not care about the audio data's timestamp except A/V synchronization. Therefore, on android, this seems not becomes a problem. But MediaDecoderStateMachine::AudioLoop() checks audio data's timestamp. Then insert the silence as the audio data's timestamp requests.

Comment 42

[Sotaro Ikeda [:sotaro]]

Therefore, this is a mp3 decoder's problem. Therefore, it seems POVB problem.

Comment 43

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Jaemin -

Per Comment#40 to Comment#42, this is a platform mp3 decoder problem, please raise a SR to QCT to check this one

Thanks

Vance

Comment 44

[Sotaro Ikeda [:sotaro]]

mp3 decoder's output was the following. 
- 0 us // frame size = 2492
- 26122 us // frame size = 4608
- 52244 us // frame size = 4608
- 78367 us // frame size = 4608
- 104489 us // frame size = 4608
- 130612 us // frame size = 4608
- 156734 us // frame size = 4608
- 182857 us // frame size = 4608

The above frame was cut by the following code in OMXCodec::read(). Then frame become like Comment 40.

>	    if (mSkipCutBuffer != NULL) {
>	        mSkipCutBuffer->submit(info->mMediaBuffer);
>	    }

Comment 45

[Sotaro Ikeda [:sotaro]]

SkipCutBuffer is created in OMXCodec::allocateBuffersOnPort().
http://androidxref.com/4.4.4_r1/xref/frameworks/av/media/libstagefright/OMXCodec.cpp#1630

Comment 46

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #44)
> 
> >	    if (mSkipCutBuffer != NULL) {
> >	        mSkipCutBuffer->submit(info->mMediaBuffer);
> >	    }

If audio data is cut in the middile, timestamp also need to be updated. But it is not updated.

Comment 47

[Sotaro Ikeda [:sotaro]]

OMXCodec and SoftMP3 seem to have problems of timestamp handling when valid data does not fill whole data.

Comment 48

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Jaemin Song from comment #34)
> I don't know why "MediaDecoderStateMachine::PlaySilence" is called.

If two consecutive AudioData blocks have a gap between them, we inject silence to ensure that the frames from the second AudioData start playing at the time the AudioData reports is its start time.

I also saw had this problem with Windows' system AAC decoder. I fixed it by capturing the timestamp reported by the system decoder on the first decoded block (at the beginning of the file, or after a seek). Otherwise I ignore the timestamp produced by the system decoder, and count the number of audio frames output by the decoder on every sample. I then calculate the timestamp of each AudioData as the timestamp of the capture point plus the number of frames decoded since capture.

Code here:
http://mxr.mozilla.org/mozilla-central/source/content/media/fmp4/wmf/WMFAudioMFTManager.cpp#235

Comment 49

[Jaemin Song]

Thank you for your description.
I understand why "PlaySilence" is called.
But I am not sure whether "PlaySilence" is necessary.

I have checked frame size using Android device.
In android device,
I found that audio frame size is same with FireFoxOS's frame size like comment 40.
As you know, android audio player is just pushing audio data to audio track.
And time stamp just refers kKeyTime(from MP3 extractor).
Although player does not add silence frame, playback is working well.

In FireFox Os,
"PlaySilence" function guarantees time stamp between audio data.
But if player does not use "PlaySilence" function like android,
playback will be operated using extractor's time stamp.
In this case, player just increases "playedFrames" count when valid data does not fill whole data.
If remove "PlaySilence" function like this,
which problems were happened?

Thanks.

Comment 50

[Sotaro Ikeda [:sotaro]]

padenot, can you answer to Comment 49?

Comment 51

[Sotaro Ikeda [:sotaro]]

Anyway, the problem happens because of incorrect timestamps of OMXCodec and SoftMP3. They have to be fixed. gecko side should not change because of it. Such thing could regress the auido playbacks.

Comment 52

[Sotaro Ikeda [:sotaro]]

Attachment: patch - OMXCodec change

The patch fixed PlaySilence() call in the start of the playback. But it seems not fix the tick noise.

Comment 53

[Sotaro Ikeda [:sotaro]]

Even with applying attachment 8490510 [details] [diff] [review], during the audio file playback, 1 frame drop sometimes happened. It seems to come from rounding error.

Comment 54

[Jaemin Song]

(In reply to Sotaro Ikeda [:sotaro] from comment #53)
> Even with applying attachment 8490510 [details] [diff] [review], during the
> audio file playback, 1 frame drop sometimes happened. It seems to come from
> rounding error.

I have checked your patch.
Patch code is updating time stamp when audio data is cut.
After applying this patch, I can confirm that "PlaySilence" is not called.
Tick noise is removed at the start position.
Because gecko side should not be changed, this patch seems be needed.

For rounding problem, could you modified "missingFrames.value() > 0" code to 
"missingFrames.value() > 1"??
In my opinion, it is necessary to alleviate the condition for rounding problem.
I could find this 1 frame drop on vorbis codec.

Thank you for your help.

Comment 55

[Seil Park]

Can you patch frameworks/av git for cherry-pick? thanks.

Comment 56

[Sotaro Ikeda [:sotaro]]

one missing frames happened based on whether conversion from frame unit to time(us) have a fraction like the following.
> (1) sample=55296 with rate-44100 -> 12538775.5 us -> 12538775 us
> (2) sample=56448 with rate-44100 -> 1280000 us

(1)'s time is 12538775 us. It's reverse conversion with UsecsToFrames() becomes 55295. One frame shorter than the original. To prevent this, Add 1 us fuzz to UsecsToFrames() seems necessary.

Comment 57

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2gv2.0 - Add Fuzz

Comment 58

[Sotaro Ikeda [:sotaro]]

By applying attachment 8490510 [details] [diff] [review] and attachment 8490897 [details] [diff] [review], intermittent 1 missingFrames during the playback disappeared. But at the start of the playback, 1 missingFrames becomes appear.

Comment 59

[Sotaro Ikeda [:sotaro]]

By the investigation, it becomes clear that 1us fuzz by attachment 8490897 [details] [diff] [review] is not enough. 2us fuzz is necessary. Normally less 1us happen by conversion from frame unit to time(us). But when frame is cut by SkipCutBuffer in OMXCodec, attachment 8490510 [details] [diff] [review] could cause less 1us. In total, less 2us could happen when SkipCutBuffer is used.

attachment 8490510 [details] [diff] [review]'s rounding error could be prevented by the following. Though, there might be a case that it's assumption do not work. Therefore it seems safer to increase fuss to 2 us.

-(1) Convert MediaBuffer's timestamp to frame unit
-(2) Convert reduced MediaBuffer's length(Byte) to frame unit.
-(3) Add (1) result and (2) result.
-(4) Convert (3) result from frame unit to time(us).
-(5) Update MediaBuffer's timestamp

(1) might not work in some cases. Therefore, attachment 8490510 [details] [diff] [review] seems safer.

Comment 60

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2gv2.0 - Add Fuzz

Change fuzz from 1us to 2us.

Comment 61

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 for b2gv2.0 - Add Fuzz

Update to a correct patch.

Comment 62

[Sotaro Ikeda [:sotaro]]

Jaemin, can you confirm the patches prevent PlaySilence() for you?

Comment 63

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - OMXCodec change

Update nits.

Comment 64

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8490941 [details] [diff] [review]
patch part 1 for b2gv2.0 - Add Fuzz

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

::: content/media/VideoUtils.cpp
@@ +24,5 @@
>  
>  // Converts from microseconds to number of audio frames, given the specified
>  // audio rate.
>  CheckedInt64 UsecsToFrames(int64_t aUsecs, uint32_t aRate) {
> +  // Add 2us fuzz. See Bug 1052206.

It would seem better to add 2us fuzz in your caller of UsecsToFrames, rather than adding fuzz to UsecsToFrames directly, as adding fuzz here affects other platforms which don't have this bug.

Comment 65

[Sotaro Ikeda [:sotaro]]

I agree, I will update the patch.

Comment 66

[Sotaro Ikeda [:sotaro]]

cpearce, during checking UsecsToFrames(), I found the following code in MediaDecoderStateMachine::SendStreamAudio(). Isn't the argument opposite(time and rate)?

>  CheckedInt64 audioWrittenOffset = UsecsToFrames(mInfo.mAudio.mRate,
>      aStream->mInitialTime + mStartTime) + aStream->mAudioFramesWritten;
>  CheckedInt64 frameOffset = UsecsToFrames(mInfo.mAudio.mRate, aAudio->mTime);

http://mxr.mozilla.org/mozilla-central/source/content/media/MediaDecoderStateMachine.cpp#303

Comment 67

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 for b2gv2.0 - Add Fuzz

Apply the comment.

Comment 68

[cajbir (:cajbir)]

Comment on attachment 8491055 [details] [diff] [review]
patch part 1 for b2gv2.0 - Add Fuzz

I defer to cpearce on the MDSM::AudioLoop code since I believe he rewrote most of that at some point.

Comment 69

[cajbir (:cajbir)]

Comment on attachment 8490974 [details] [diff] [review]
patch part 2 - OMXCodec change

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

Can you add comments explaining what the code is supposed to be doing and why? I'll review when you've done that so I can match that the code does what you intend. Otherwise I have no real idea what this is for since I'm not overly familar with OMXCodec stuff.

Comment 70

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Sotaro Ikeda [:sotaro] from comment #66)
> cpearce, during checking UsecsToFrames(), I found the following code in
> MediaDecoderStateMachine::SendStreamAudio(). Isn't the argument
> opposite(time and rate)?
> 
> >  CheckedInt64 audioWrittenOffset = UsecsToFrames(mInfo.mAudio.mRate,
> >      aStream->mInitialTime + mStartTime) + aStream->mAudioFramesWritten;
> >  CheckedInt64 frameOffset = UsecsToFrames(mInfo.mAudio.mRate, aAudio->mTime);
> 
> http://mxr.mozilla.org/mozilla-central/source/content/media/
> MediaDecoderStateMachine.cpp#303

Yes, that's incorrect. Thanks! I'd guess it's gone unnoticed for so long because the time parameters didn't overflow uint32_t much...

Comment 71

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #70)
> (In reply to Sotaro Ikeda [:sotaro] from comment #66)
> > cpearce, during checking UsecsToFrames(), I found the following code in
> > MediaDecoderStateMachine::SendStreamAudio(). Isn't the argument
> > opposite(time and rate)?
> > 
> > >  CheckedInt64 audioWrittenOffset = UsecsToFrames(mInfo.mAudio.mRate,
> > >      aStream->mInitialTime + mStartTime) + aStream->mAudioFramesWritten;
> > >  CheckedInt64 frameOffset = UsecsToFrames(mInfo.mAudio.mRate, aAudio->mTime);
> > 
> > http://mxr.mozilla.org/mozilla-central/source/content/media/
> > MediaDecoderStateMachine.cpp#303
> 
> Yes, that's incorrect. Thanks! I'd guess it's gone unnoticed for so long
> because the time parameters didn't overflow uint32_t much...

I'll fix this.

Comment 72

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #71)
> (In reply to Chris Pearce (:cpearce) from comment #70)
> > (In reply to Sotaro Ikeda [:sotaro] from comment #66)
> > > cpearce, during checking UsecsToFrames(), I found the following code in
> > > MediaDecoderStateMachine::SendStreamAudio(). Isn't the argument
> > > opposite(time and rate)?
> > > 
> > > >  CheckedInt64 audioWrittenOffset = UsecsToFrames(mInfo.mAudio.mRate,
> > > >      aStream->mInitialTime + mStartTime) + aStream->mAudioFramesWritten;
> > > >  CheckedInt64 frameOffset = UsecsToFrames(mInfo.mAudio.mRate, aAudio->mTime);
> > > 
> > > http://mxr.mozilla.org/mozilla-central/source/content/media/
> > > MediaDecoderStateMachine.cpp#303
> > 
> > Yes, that's incorrect. Thanks! I'd guess it's gone unnoticed for so long
> > because the time parameters didn't overflow uint32_t much...
> 
> I'll fix this.

bug 1068970

Comment 73

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8491055 [details] [diff] [review]
patch part 1 for b2gv2.0 - Add Fuzz

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

::: content/media/MediaDecoderStateMachine.cpp
@@ +880,5 @@
>      const AudioData* s = AudioQueue().PeekFront();
>  
>      // Calculate the number of frames that have been pushed onto the audio
>      // hardware.
> +    CheckedInt64 playedFrames = UsecsToFrames(audioStartTime + AUDIO_FUZZ_USECS, rate) +

MediaDecoderStateMachine::AudioLoop() does not exist anymore in m-c, will you need to land this fix there too, or will this fix only exist on some other branch? i.e. is this fix B2G only, and not going to land on branches that affect Desktop Firefox?

Comment 74

[Jaemin Song]

(In reply to Sotaro Ikeda [:sotaro] from comment #62)
> Jaemin, can you confirm the patches prevent PlaySilence() for you?

I have checked your two patches.
As a result of test,
PlaySilence() was not called and silence sound also was not added.
And 1 frame drop also was not occurred.
Problem is fixed.

Thanks.

Comment 75

[Sotaro Ikeda [:sotaro]]

(In reply to Chris Pearce (:cpearce) from comment #73)
> MediaDecoderStateMachine::AudioLoop() does not exist anymore in m-c, will
> you need to land this fix there too, or will this fix only exist on some
> other branch? i.e. is this fix B2G only, and not going to land on branches
> that affect Desktop Firefox?

I want to fix also on master. I will create a patch for master.

Comment 76

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 for master - Add Fuzz

Comment 77

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - OMXCodec change

Add comments.

Comment 78

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8491557 [details] [diff] [review]
patch part 1 for master - Add Fuzz

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

Matthew: Sotaro has identified that some B2G devices' system decoders have rounding errors in their timestamp calculations, and that causes 1 or two frame gaps when decoding audio, which causes us to play silence, and thus glitch.

What do you think about Sotaro's patch, and my suggestion?

::: content/media/AudioSink.cpp
@@ +165,5 @@
>        NS_WARNING("Int overflow adding in AudioLoop");
>        break;
>      }
>  
>      if (missingFrames.value() > 0) {

I think rather than adding fuzz to timestamps, what we should do instead is:

 if (missingFrames.value() > AUDIO_FUZZ_USECS) {
   // ... PlaySilence()....

Comment 79

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

Comment on attachment 8491557 [details] [diff] [review]
patch part 1 for master - Add Fuzz

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

::: content/media/AudioSink.cpp
@@ +154,5 @@
>      // audio hardware, so we can play across the gap.
>      // Calculate the timestamp of the next chunk of audio in numbers of
>      // samples.
>      NS_ASSERTION(AudioQueue().GetSize() > 0, "Should have data to play");
> +    CheckedInt64 sampleTime = UsecsToFrames(AudioQueue().PeekFront()->mTime + AUDIO_FUZZ_USECS, mInfo.mRate);

Should we add AUDIO_FUZZ_USECS here? Since you add AUDIO_FUZZ_USECS to both |AudioQueue().PeekFront()->mTime| and |mStartTime|, the difference remains the same.

@@ +165,5 @@
>        NS_WARNING("Int overflow adding in AudioLoop");
>        break;
>      }
>  
>      if (missingFrames.value() > 0) {

Agree. |sampleTime| and |missingFrames| should stay with their original meaning.

Comment 80

[Matthew Gregan [:kinetik]]

Comment on attachment 8491557 [details] [diff] [review]
patch part 1 for master - Add Fuzz

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

::: content/media/AudioSink.cpp
@@ +165,5 @@
>        NS_WARNING("Int overflow adding in AudioLoop");
>        break;
>      }
>  
>      if (missingFrames.value() > 0) {

Yeah, I would prefer this also.

Longer term, it'd be nicer to do all of these calculations in frame units without going back and forth between frames and usecs, but that'd require more significant changes, and we'll still need to deal with any timecode rounding errors on the AudioData start time produced upstream by the decoder.

Comment 81

[Sotaro Ikeda [:sotaro]]

The intent of adding fuzz to time is that android stagefright's audio data time could have from 0 to -2us variance because of rounding error. It increase the fuzz more than necessary to fix the bug. It is from 2us to 22us. It is the reason why I did not choose the following in attachment 8491557 [details] [diff] [review]. 

> if (missingFrames.value() > 0) {

Comment 82

[Sotaro Ikeda [:sotaro]]

> It is from 2us to 22us.

Correction:
  If we choose frame count. The fuzz become 2us to 22us.

Comment 83

[Sotaro Ikeda [:sotaro]]

(In reply to Matthew Gregan [:kinetik] from comment #80)
> ::: content/media/AudioSink.cpp
> @@ +165,5 @@
> >        NS_WARNING("Int overflow adding in AudioLoop");
> >        break;
> >      }
> >  
> >      if (missingFrames.value() > 0) {
> 
> Yeah, I would prefer this also.

I am going to apply it in a next patch.

Comment 84

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 for master - Add Fuzz

Apply the comment.

Comment 85

[cajbir (:cajbir)]

Comment on attachment 8491613 [details] [diff] [review]
patch part 2 - OMXCodec change

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

::: media/libstagefright/OMXCodec.cpp
@@ +4295,5 @@
>      info->mStatus = OWNED_BY_CLIENT;
>  
>      info->mMediaBuffer->add_ref();
>      if (mSkipCutBuffer != NULL) {
> +        // Get an original MediaBuffer length.

remove 'an'

@@ +4302,2 @@
>          mSkipCutBuffer->submit(info->mMediaBuffer);
> +        // Get a MediaBuffer length after SkipCutBuffer::submit() call. 

Remove 'a' and trailing space at end.

@@ +4304,5 @@
> +        size_t result_len = info->mMediaBuffer->range_length();
> +        int32_t numchannels = 0;
> +        int32_t audioSampleRate = 0;
> +        int64_t timeUs = 0;
> +        // If SkipCutBuffer cut MediaBuffer data, progress its time the priod

spelling of 'period'

@@ +4311,5 @@
> +            mOutputFormat->findInt32(kKeyChannelCount, &numchannels) &&
> +            mOutputFormat->findInt32(kKeySampleRate, &audioSampleRate) &&
> +            info->mMediaBuffer->meta_data()->findInt64(kKeyTime, &timeUs)) {
> +            if (numchannels > 0 && audioSampleRate > 0) {
> +                // Calculate a number of cut frames.

Remove 'a'

Comment 86

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 for b2gv2.0 - Add Fuzz

A patch for b2gv2.0. Carry "r=kinetik".

Comment 87

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 for KK - OMXCodec change

Apply the comment. Carry "r=cajbir".

Comment 88

[Sotaro Ikeda [:sotaro]]

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

Comment 89

[Sotaro Ikeda [:sotaro]]

https://github.com/mozilla-b2g/platform_frameworks_av/commit/e796bb13b65fa9add073f92196e3472a8783f1e0

Comment 90

[Sotaro Ikeda [:sotaro]]

Hi diego, can attachment 8493313 [details] [diff] [review] be merged to caf?

Comment 91

[Sotaro Ikeda [:sotaro]]

Hi james.zhang, can I have a feedback to attachment 8493313 [details] [diff] [review]?

Comment 92

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 93

[Ming]

Hi sotaro ,we are on v1.4, so should we need  patch attachment 8493305 [details] [diff] [review] for v1.4?

Comment 94

[Rachelle Yang [:ryang][[e-mail]]]

denominate 2.0?. Partner ignored this bug.

Comment 95

[Sotaro Ikeda [:sotaro]]

Rachelle, I do not understand why you set this bug as WONTFIX. To handle this bug as WONTFIX means, Firefox OS mp3 playback could have a tick noise of it's playback start.

Comment 96

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #95)
> Rachelle, I do not understand why you set this bug as WONTFIX. To handle
> this bug as WONTFIX means, Firefox OS mp3 playback could have a tick noise
> of it's playback start.

Can you explain more?

Comment 97

[Sotaro Ikeda [:sotaro]]

Anyway, part of the fix is already committed to m-c WONTFIX is incorrect state.

Comment 98

[Sotaro Ikeda [:sotaro]]

Can you provide a feedback to attachment 8493313 [details] [diff] [review]?

Comment 99

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #95)
> Rachelle, I do not understand why you set this bug as WONTFIX. To handle
> this bug as WONTFIX means, Firefox OS mp3 playback could have a tick noise
> of it's playback start.

I do not understand the decision of permit such defect forever on firefox os.

Comment 100

[Sotaro Ikeda [:sotaro]]

(In reply to Ming from comment #93)
> Hi sotaro ,we are on v1.4, so should we need  patch attachment 8493305 [details] [diff] [review]
> [details] [diff] [review] for v1.4?

If the bug is set to b2g-v1.4+. It need to be landed on v1.4. But this bug does not flagged as b2g-v1.4+.

Comment 101

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #99)
> (In reply to Sotaro Ikeda [:sotaro] from comment #95)
> > Rachelle, I do not understand why you set this bug as WONTFIX. To handle
> > this bug as WONTFIX means, Firefox OS mp3 playback could have a tick noise
> > of it's playback start.
> 
> I do not understand the decision of permit such defect forever on firefox os.

If it does not get the b2g-v2.0+, such bug should be set to backlog.

Comment 102

[Sotaro Ikeda [:sotaro]]

Bug 1009410 is set as feature of b2g-v2.1. If it is completed, MediaCodec is used for video/audio decoding instead of OMXCodec. I do not know Bug 1009410 will be actually completed in b2g-v2.1.

Comment 103

[Diego Wilson [:diego]]

(In reply to Sotaro Ikeda [:sotaro] from comment #98)
> Can you provide a feedback to attachment 8493313 [details] [diff] [review]?

Sotaro,

Can you please summarize what this attachment does? Do you want it to be upstreamed to CAF?

Comment 104

[Sotaro Ikeda [:sotaro]]

(In reply to Diego Wilson [:diego] from comment #103)
> Can you please summarize what this attachment does?

OMXCodec cut start of audio data. But it does not update a timestamp of MediaBuffer.
AudioSink::AudioLoop() checks consistency of played audio frame number and audio data's timestamp. If AudioSink::AudioLoop() detects the inconsistency during the check, AudioSink::AudioLoop() insert PlaySilence() to adjust the timestamp inconsistency. Because of this mechanism, when starting mp3 file playback, AudioSink::AudioLoop() detects timestamp inconsistency and insert PlaySilence(). It could be heard as tick noise in some mp3 files.

The patch updates timestamp of MediaBuffer correctly.

Comment 105

[Sotaro Ikeda [:sotaro]]

(In reply to Diego Wilson [:diego] from comment #103)
> 
> Can you please summarize what this attachment does? Do you want it to be
> upstreamed to CAF?

I do not think this bug have high priority to uplift. But I do not want that this bug exists forever on b2g. If we move from OMXCodec to MediaCodec, we might need to fix only on Bug 1072295 on CAF.

Comment 106

[Sotaro Ikeda [:sotaro]]

brsun, do you know when gecko start to use MediaCodec for audio decoding?

Comment 107

[Diego Wilson [:diego]]

(In reply to Sotaro Ikeda [:sotaro] from comment #104)
> (In reply to Diego Wilson [:diego] from comment #103)
> > Can you please summarize what this attachment does?
> 
> OMXCodec cut start of audio data. But it does not update a timestamp of
> MediaBuffer.
> AudioSink::AudioLoop() checks consistency of played audio frame number and
> audio data's timestamp. If AudioSink::AudioLoop() detects the inconsistency
> during the check, AudioSink::AudioLoop() insert PlaySilence() to adjust the
> timestamp inconsistency. Because of this mechanism, when starting mp3 file
> playback, AudioSink::AudioLoop() detects timestamp inconsistency and insert
> PlaySilence(). It could be heard as tick noise in some mp3 files.
> 
> The patch updates timestamp of MediaBuffer correctly.

I see. Passing along feedback? to Bhargav who's been done more audio mm work than me.

@Sotaro, just FYI if this patch is to be upstreamed to CAF it must first land on mozilla's project [1]

[1] http://git.mozilla.org/b2g/platform_frameworks_av.git

Comment 108

[Diego Wilson [:diego]]

Comment on attachment 8493313 [details] [diff] [review]
patch part 2 for KK - OMXCodec change

(In reply to Sotaro Ikeda [:sotaro] from comment #104)
> (In reply to Diego Wilson [:diego] from comment #103)
> > Can you please summarize what this attachment does?
> 
> OMXCodec cut start of audio data. But it does not update a timestamp of
> MediaBuffer.
> AudioSink::AudioLoop() checks consistency of played audio frame number and
> audio data's timestamp. If AudioSink::AudioLoop() detects the inconsistency
> during the check, AudioSink::AudioLoop() insert PlaySilence() to adjust the
> timestamp inconsistency. Because of this mechanism, when starting mp3 file
> playback, AudioSink::AudioLoop() detects timestamp inconsistency and insert
> PlaySilence(). It could be heard as tick noise in some mp3 files.
> 
> The patch updates timestamp of MediaBuffer correctly.

Bhargav, Sotaro wants to upstream this patch to CAF. It's explained above. What do you think?

Comment 109

[Sotaro Ikeda [:sotaro]]

(In reply to Diego Wilson [:diego] from comment #107)
> 
> @Sotaro, just FYI if this patch is to be upstreamed to CAF it must first
> land on mozilla's project [1]
> 
> [1] http://git.mozilla.org/b2g/platform_frameworks_av.git

It is already landed by the following.
https://github.com/mozilla-b2g/platform_frameworks_av/commit/e796bb13b65fa9add073f92196e3472a8783f1e0

Comment 110

[Rachelle Yang [:ryang][[e-mail]]]

Hi Sotaro, thanks for your work!Got it!

Comment 111

[Bruce Sun [:brsun]]

(In reply to Sotaro Ikeda [:sotaro] from comment #106)
> brsun, do you know when gecko start to use MediaCodec for audio decoding?

For MSE, MediaCodec should be already used.

For other cases, it would be better to solve all depending bugs of bug 1033900 before enabling MediaCodec by default (bug 1033935).
I don't have confidence to declare the timeline now, but hopefully it might be the next FxOS release on master.

Comment 112

[Diego Wilson [:diego]]

(In reply to Sotaro Ikeda [:sotaro] from comment #109)
> (In reply to Diego Wilson [:diego] from comment #107)
> > 
> > @Sotaro, just FYI if this patch is to be upstreamed to CAF it must first
> > land on mozilla's project [1]
> > 
> > [1] http://git.mozilla.org/b2g/platform_frameworks_av.git
> 
> It is already landed by the following.
> https://github.com/mozilla-b2g/platform_frameworks_av/commit/
> e796bb13b65fa9add073f92196e3472a8783f1e0

OK, I'll work on pulling it into CAF. Can you please open a new bug for tracking?

Comment 113

[Sotaro Ikeda [:sotaro]]

(In reply to Diego Wilson [:diego] from comment #112)
> 
> OK, I'll work on pulling it into CAF. Can you please open a new bug for
> tracking?

In this bug, I forget to think about ACodec, it might be better to handle it in 1072295.

Comment 114

[Diego Wilson [:diego]]

(In reply to Sotaro Ikeda [:sotaro] from comment #113)
> (In reply to Diego Wilson [:diego] from comment #112)
> > 
> > OK, I'll work on pulling it into CAF. Can you please open a new bug for
> > tracking?
> 
> In this bug, I forget to think about ACodec, it might be better to handle it
> in 1072295.

Do you still want me to pull in attachment 8493313 [details] [diff] [review] to CAF then?

Comment 115

[Sotaro Ikeda [:sotaro]]

(In reply to Diego Wilson [:diego] from comment #114)
> 
> Do you still want me to pull in attachment 8493313 [details] [diff] [review]
> to CAF then?

I do not want it now, because this bug is not flagged to b2g-v2.0+. Then instead, I want to pull bug 1072295 fix to CAF, when the bug is fixed.

Comment 116

[bhargavg1]

Comment on attachment 8493313 [details] [diff] [review]
patch part 2 for KK - OMXCodec change

-ni as per comment 115

Comment 117

[Diego Wilson [:diego]]

(In reply to Sotaro Ikeda [:sotaro] from comment #115)
> (In reply to Diego Wilson [:diego] from comment #114)
> > 
> > Do you still want me to pull in attachment 8493313 [details] [diff] [review]
> > to CAF then?
> 
> I do not want it now, because this bug is not flagged to b2g-v2.0+. Then
> instead, I want to pull bug 1072295 fix to CAF, when the bug is fixed.

I see. Please ni? me when you're ready to involve CAF.

Comment 118

[Sotaro Ikeda [:sotaro]]

Okey :-)

Comment 119

[Jaemin Song]

Dear Sotaro,

For v2.0, we need this patch(patch part 1 for b2gv2.0 - Add Fuzz).
But this patch is not landed on v2.0 branch.
(landed on master branch only. https://hg.mozilla.org/mozilla-central/rev/2d6ffa903e38)

And below patch also is not landed on v2.0 branch.
https://github.com/mozilla-b2g/platform_frameworks_av/commit/e796bb13b65fa9add073f92196e3472a8783f1e0
This patch was included on b2g_kk_3.5 branch only.

Could you include these patches to v2.0 branch?

Thanks.

Comment 120

[Jaemin Song]

Dear Sotaro,

I have detected another problem.
After applying your patch, 
when confirming this problem using new attached MP3 file,
problem is also detected.
In below case, "SkipCutBuffer" was not created.
Because new attached file does not have "kKeyEncoderDelay", "kKeyEncoderPadding" value.

[Reproduce step]
1. To play new attached mp3 file on non-offload mode, please connect BT.
   (Or "adb shell setprop audio.offload.disable 1")
2. Play new attached mp3 file.
3. If you start playback or operate seek function while playback,
   you can find "Play Silence" function is called.

Because of this, whenever operating seek function,
music was being chopped.
Please confirm this case.

Thanks.

Comment 121

[Jaemin Song]

Attachment: new_attached_file.mp3

new attached file.

Comment 122

[Sotaro Ikeda [:sotaro]]

Jaemin, this is another problem, can you create a new bug for it?

Comment 123

[Jaemin Song]

(In reply to Sotaro Ikeda [:sotaro] from comment #122)
> Jaemin, this is another problem, can you create a new bug for it?

OK, I'll create new bug case.

And for v2.0 branch,
please confirm comment 119.

Thanks.

Comment 124

[Jaemin Song]

Dear Sotaro,

Please check comment 119 to uplift patch to V2.0.

And comment 120's root cause was not similar with this issue.
Please ignore comment 120.

Thanks.

Comment 125

[Sotaro Ikeda [:sotaro]]

[Blocking Requested - why for this release]: Blocks 2.5+ bug(Bug 1148049).

Comment 126

[Sotaro Ikeda [:sotaro]]

I landed fix to mozilla-b2g/platform_frameworks_av. But flame-kk build refer to caf repository and the tag does not include the fix. Therefore, the fix was not applied to flame-kk.