1044772 - [B2G] OmxDecoder::ProcessCachedData would go through the whole mp3 when try to get the duration

Status: RESOLVED INVALID

(Core :: Audio/Video, defect)

Description

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

When I put a large mp3 to test the duration, 
I found the OmxDecoder::ProcessCachedData would go through the whole mp3 file. 
If we already got the mp3 duratuon, we can avoid some effort of reading data from cache and processing it.

Comment 1

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Enable media codec would make this issue disappear.

Comment 2

[Sotaro Ikeda [:sotaro]]

rlin, if MediaCodec is used, does it get correct mp3 duration for VBR mp3? It it is not, same thing becomes necessary also to MediaCodec side...

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> rlin, if MediaCodec is used, does it get correct mp3 duration for VBR mp3?
> It it is not, same thing becomes necessary also to MediaCodec side...

If stagefright's MediaExtractor is used for mp3 decoding, the MediaExtractor has the duraiton problme for mp3 VBR.

Comment 4

[Sotaro Ikeda [:sotaro]]

The VBR related code was originally added by Bug 831224.

Comment 5

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

The long term plan for MP3 is to write an MP3Reader, similar to the MP4Reader, which uses our own MP3FrameParser to demux the MP3 stream and calculate the duration, and uses the PlatformDecoderModule/MediaDataDecoder interface to use the system decoders to decode MP3 packets.

MediaOmxReader already uses MP3FrameParser to calculate the duration of the MP3 stream, why is Omx trying to calculate the duration?

Comment 6

[Sotaro Ikeda [:sotaro]]

(In reply to Chris Pearce (:cpearce) from comment #5)
> The long term plan for MP3 is to write an MP3Reader, similar to the
> MP4Reader, which uses our own MP3FrameParser to demux the MP3 stream and
> calculate the duration, and uses the PlatformDecoderModule/MediaDataDecoder
> interface to use the system decoders to decode MP3 packets.
> 
> MediaOmxReader already uses MP3FrameParser to calculate the duration of the
> MP3 stream, why is Omx trying to calculate the duration?

On MediaOmxReader, stagefright's MediaExtractor is used for parsing mp3 for playback, it does not calculate VBR mp3's duration correctly. On stagefright, it seems intentionally decided to prevent full mp3 file scan. As a side effect, VBR mp3's duration becomes inaccurate.

Then on MediaOmxReader, MP3FrameParser use used to update the duration on mp3. But it causes performance problem.

Comment 7

[Sotaro Ikeda [:sotaro]]

From it, it seems that if want to get accurate vbr mp3's duration, we need to scan all mp3 file.

Comment 8

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

OK, so this shouldn't be a issue, right? 
Let me close it and thanks for clarify.

Comment 9

[Sotaro Ikeda [:sotaro]]

(In reply to Randy Lin [:rlin] - Be Mozillian on 2015/01/26 from comment #8)
> OK, so this shouldn't be a issue, right? 
> Let me close it and thanks for clarify.

It is implementation trade-off. If we want to get correct duration for VBR mp3, it seems unavoidable.

On the other side, if we enable MediaCodec's playback for mp3 in current gecko, VBR mp3 seems to becomes in accurate.