1178214 - [Flame][Video]With some special videos in device, when user opens Video app, it exits automatically.

Status: VERIFIED FIXED

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

Description

[SandKing]

Attachment: Bug log: logcat_0203.txt

[1.Description]:
[Flame][v3.0][Video]There are some special videos(MP4 file with 1080P) in device. When user opens Video, the Video app will exit automatically.
See video:0203.3GP
See log:"logcat_0203.txt"
Found Time:02:03
See special video: Test video.MP4

[2.Testing Steps]: 
Premise: Have some special video(1080P.Mp4 file) in device.
1. Launch Video.
2. Wait a minute.

[3.Expected Result]: 
2. The Video app should not load special videos(MP4 file with 1080P)  and should not exit.

[4.Actual Result]: 
2. The Video app will exit automatically.

[5.Reproduction build]: 
Device:Flame 2.2 (Unaffected)
Build ID               20150628002505
Gaia Revision          0179935627012dfde3ca036c9a71035be463b7ad
Gaia Date              2015-06-26 21:13:44
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/35e09270da3a
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150628.035537
Firmware Date          Sun Jun 28 03:55:48 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 build (Affected)
Build ID               20150628160206
Gaia Revision          8a1e4ae522c121c5cacd39b20a5386ec9055db82
Gaia Date              2015-06-25 21:58:25
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/eaf4f9b45117
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150628.193057
Firmware Date          Sun Jun 28 19:31:08 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5_2.2 build (Unaffected)
Build ID               20150626162505
Gaia Revision          0179935627012dfde3ca036c9a71035be463b7ad
Gaia Date              2015-06-26 21:13:44
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/330f52ef6a2d
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150626.195122
Firmware Date          Fri Jun 26 19:51:42 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5_3.0 build (Unaffected)
Build ID               20150628160206
Gaia Revision          8a1e4ae522c121c5cacd39b20a5386ec9055db82
Gaia Date              2015-06-25 21:58:25
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/eaf4f9b45117
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150628.192615
Firmware Date          Sun Jun 28 19:26:32 EDT 2015
Bootloader             HHZ12f

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

Comment 1

[SandKing]

Attachment: Bug video: 0203.3GP

Comment 2

[SandKing]

Attachment: Special video: Test video.mp4

Comment 3

[Hermes Cheng (inactive after July 27, 2015)]

No-jun, could you help?

Comment 4

[No-Jun Park [:njpark]]

Sandking, I'm wondering whether this is an LMK issue, since it only happens in 3.0.  If you up the memory of the flame device to 512 or 1024, does this still happen?

Comment 5

[SandKing]

Hi No-Jun,
I can not reproduce this bug when I up the memory of the flame device to 512M or 1024M

Flame v3.0:
Build ID               20150705010207
Gaia Revision          dc6c18c0dea7af3c40bfff86c530fd877d899dc4
Gaia Date              2015-07-04 01:35:20
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/136c41fca853
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150705.044135
Firmware Date          Sun Jul  5 04:41:46 EDT 2015
Bootloader             L1TC000118D0

Comment 6

[No-Jun Park [:njpark]]

Okay, so it looks like it's an LMK error, like bug 1038459.  Looks like this came back.

Comment 7

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:

Noming to start discussion of 'what do we do when the 319MB device LMK is caused by large media file?'

Comment 8

[Blake Wu [:bwu][:blakewu]]

Ni Munro since IIRC there is a bug similar with this one.

Comment 10

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

The symptom only occurs while parsing thumbnail of a 1080p resolution media file.
Actually, Flame's hardware codec bandwidth can only afford playing 720p resolution smoothly.
When you set the memory to 512, you should notice the crash is gone but the 1080p video is not played smoothly.

Below is my opinion.

1. In this LMK case, we should prevent video app from parsing any 1080p media file on Flame since its CPU(Qualcomm msm8610) spec already indicate its capability is 720p.

2. If we want to prevent video app being killed even trying allocate a 1080p decoder, we should use preserved ion memory for media, then it will not consume all system memory and trigger LMK. But the memory layout change need to be done in partner side.

Comment 11

[Michael:mtreese]

changed to 2.5+  due to crash with less than 512k memory. Should detect 1080p and throw an error instead of crashing.

Comment 12

[Blake Wu [:bwu][:blakewu]]

(In reply to Munro Chiang [:munro_chiang] from comment #10)
> The symptom only occurs while parsing thumbnail of a 1080p resolution media
> file.
> Actually, Flame's hardware codec bandwidth can only afford playing 720p
> resolution smoothly.
> When you set the memory to 512, you should notice the crash is gone but the
> 1080p video is not played smoothly.
> 
> Below is my opinion.
> 
> 1. In this LMK case, we should prevent video app from parsing any 1080p
> media file on Flame since its CPU(Qualcomm msm8610) spec already indicate
> its capability is 720p.
> 
> 2. If we want to prevent video app being killed even trying allocate a 1080p
> decoder, we should use preserved ion memory for media, then it will not
> consume all system memory and trigger LMK. But the memory layout change need
> to be done in partner side.

Since we cannot control the app/website side to play 1080P file, do you think we could use the same way to block the app as OMXCodecProxy.cpp[1] in our current codes? To return an error to apps. 

[1]https://dxr.mozilla.org/mozilla-central/source/dom/media/omx/OMXCodecProxy.cpp?from=OMXCodecProxy.cpp&offset=0#167

Comment 13

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: WIP-bug-fix-1178214.patch

According to Qualcomm msm8610 spec, Flame's hardware codec can only afford playing 720p resolution smoothly (@30fps). However, if upstream try to allocate a 1080p decoder, video codec driver won't return error but only provide low performance / fps. To prevent poor fps / performance observed, we should prevent generating thumbnail for these unsupported resolution video.

Comment 14

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8651699 [details] [diff] [review]
WIP-bug-fix-1178214.patch

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

If the restriction is caused by hardware limitation, it might be better to use android's system properties like 'ro.moz.omx.hw.max_height' and 'ro.moz.omx.hw.max_width'.

https://dxr.mozilla.org/mozilla-central/source/dom/media/omx/OMXCodecProxy.cpp?offset=0#157

Comment 15

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Thanks for the comment! attachment 8651699 [details] [diff] [review] uses these two properties 'ro.moz.omx.hw.max_height' and 'ro.moz.omx.hw.max_width'.
If these two properties are not defined, it will use MAX_VIDEO_HEIGHT and MAX_VIDEO_WIDTH.

Comment 16

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8651699 [details] [diff] [review]
WIP-bug-fix-1178214.patch

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

Sorry, I wrote wrong comment in the patch. I wanted to comment that MediaCodecReader case seems not handled by the patch.

Comment 17

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Benjamin, is there any use case we will use MediaCodecReader?

Comment 18

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Benjamin will remove MediaCodecReader in Bug 1198576.
GonkVideoDecoderManager should cover all use case now.

Comment 19

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8651699 [details] [diff] [review]
WIP-bug-fix-1178214.patch

Looks good!

Comment 20

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

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

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/5c66ee30f6d6

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5c66ee30f6d6

Comment 23

[SandKing]

This bug has been verified as pass on latest Flame v2.5 by the STR in Comment 0.
Repro rate: 0/10

Actual result:
The Video app does not load special videos(MP4 file with 1080P)and the page stays at video list view

Flame v2.5(Pass):
Build ID               20150830150218
Gaia Revision          31e595f86f6bf159b3a9a46816a6ac00a55ca9f9
Gaia Date              2015-08-30 00:42:30
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2ad5077d86ba81b667de45ccc986dbd2ce633cc4
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150830.184031
Firmware Date          Sun Aug 30 18:40:47 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0