Status

()

defect
--
major
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: james.zhang, Assigned: jesse.ji)

Tracking

({smoketest})

28 Branch
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

Details

(Whiteboard: [tarako_only], 1.3tarakorun1)

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch tavarua_fm_support.patch (obsolete) — Splinter Review
Trout FM is differnet from beken and any other FM hardware. It must denpend on pcm audio hal interface. Please review this patch, it need merge into v1.3t.
Jesse provide this patch.
Assignee: nobody → jesse.ji
Flags: needinfo?(ttsai)
Flags: needinfo?(styang)
Attachment #8388458 - Flags: review?(ttsai)
Attachment #8388458 - Flags: review?(mwu)
Hi Michael, would you mind to take a look on this patch? thanks!
Flags: needinfo?(styang)
Attachment #8388458 - Flags: review?(ttsai)
Flags: needinfo?(ttsai)
Only tarako need this patch, fugu and dolphin need not this patch.
So we only need merge it into v1.3t.
Need merge into v1.3t for tarako only.
blocking-b2g: --- → 1.3T?
Whiteboard: [tarako_only]
blocking-b2g: 1.3T? → 1.3T+
Component: Hardware → Hardware Abstraction Layer (HAL)
Product: Firefox OS → Core
Version: unspecified → 28 Branch
Comment on attachment 8388458 [details] [diff] [review]
tavarua_fm_support.patch

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

Sorry, I don't think this patch belongs in gecko. Instead, I believe this thread should be started when AudioSystem::setDeviceConnectionState is called. When gecko wants to turn FM on, it will notify the audio system using that call, at which point an appropriate thread can be started for FM. One way to do this is to change setDeviceConnectionState inside hardware/libhardware_legacy/audio/AudioPolicyManagerBase.cpp. The only tricky thing is that the bit gecko uses to indicate the FM radio device is not the same as the ones that this platform uses. On gecko, AUDIO_DEVICE_OUT_FM = 0x80000. It looks like this code already handles FM devices, but it doesn't know about the different bit that gecko uses. Let me know if you need any help with this.
Attachment #8388458 - Flags: review?(mwu) → review-
Flags: needinfo?(jesse.ji)
If I put the code in 'AudioPolicyManagerBase.cpp', The libhardware_legacy library will depend on gecko library due to gecko AudioStream references, otherwise gecko AudioChannelManager can't manager the fm audio stream.
Flags: needinfo?(jesse.ji)
Flags: needinfo?(mwu)
Hi Michael,

   Can you give some libhardware_legacy patch for tarako?

   We need your help.
I'll take this.
Assignee: jesse.ji → mwu
Flags: needinfo?(mwu)
Duplicate of this bug: 985315
Attachment #8388458 - Attachment is obsolete: true
James, let me know if this patch works for you.
Flags: needinfo?(james.zhang)
looks good to me , have not tested it. The only defect may be that the code does not delete FMRadioPlayer object when fm stopped.
Flags: needinfo?(mwu)
Jess will commit this patch on my side.
Flags: needinfo?(james.zhang)
do we need this patch on the 1.3T branch? Thanks if so, can you commit to 1.3T? and fixed on  status-b2g-v1.3T? Thanks
Flags: needinfo?(jesse.ji)
(In reply to Joe Cheng [:jcheng] from comment #14)
> do we need this patch on the 1.3T branch? Thanks if so, can you commit to
> 1.3T? and fixed on  status-b2g-v1.3T? Thanks

Joe - the patch needs to be a r+ before we land it.
(In reply to Joe Cheng [:jcheng] from comment #14)
> do we need this patch on the 1.3T branch? Thanks if so, can you commit to
> 1.3T? and fixed on  status-b2g-v1.3T? Thanks

This patch applies to a spreadtrum side repo.
Flags: needinfo?(jesse.ji)
(In reply to Jesse from comment #12)
> looks good to me , have not tested it. The only defect may be that the code
> does not delete FMRadioPlayer object when fm stopped.

We actually don't need to dynamically allocate the FMRadioPlayer object at all. It only has three fields, so it might as well be statically allocated.
Flags: needinfo?(mwu)
Assignee: mwu → jesse.ji
Fixed on my side.

Michael, thank you very much for your kindly help.


commit 2222043791e7616bb09eb1c572f99981b9de195f
Author: jesse.ji <jesse.ji@spreadtrum.com>
Date:   Fri Mar 21 11:01:17 2014 +0800

    Bug #285579  Move trout FM support in hardware repo, not in gecko.
    
    [bug number  ] 285579
    [root cause  ]
    [changes     ]
    [side effects]
    [reviewers   ]
    [self test   ] self test
    
    Change-Id: I38bb01aa8fa6d9c0ca6ba83b82cc5321c5113f4c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [tarako_only] → [tarako_only], 1.3tarakorun1
You need to log in before you can comment on or make changes to this bug.