Closed Bug 877193 Opened 11 years ago Closed 10 years ago

Reduce memory usage for RTSP streaming

Categories

(Firefox OS Graveyard :: RTSP, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(tracking-b2g:backlog, b2g-v2.0 verified)

VERIFIED FIXED
1.4 S6 (25apr)
tracking-b2g backlog
Tracking Status
b2g-v2.0 --- verified

People

(Reporter: bechen, Assigned: ethan)

References

Details

(Whiteboard: [p=1])

Attachments

(1 file, 1 obsolete file)

This is a follow bug for bug 831645.

At bug 831645, I add a new class |RtspTrackBuffer| for buffering the audio/video frame data in RtspMediaResource. 
These three define values controls the buffer size we actually allocate.
#define BUFFER_SLOT_NUM 128
#define BUFFER_SLOT_DEFAULT_SIZE 1024
#define BUFFER_SLOT_MAX_SIZE 8192
That means the memory usage is 128k at least to 1024k bytes.

Maybe we should move these values to Preference so that we can change them apply to different devices (various HW decode ability)?
And also modify the heuristic that I wrote in bug 831645: (frame width)*(frame height)

Hi Sotaro:
Do we have a way to know the HW decode ability such as max resolution?
Flags: needinfo?(sotaro.ikeda.g)
media_profiles.xml could provide the information. But it often does not provide enough information for it.
Flags: needinfo?(sotaro.ikeda.g)
Blocks: b2g-RTSP-1.3
blocking-b2g: --- → 1.3?
Whiteboard: [ft:ril]
Blocks: backlog-RIL/Net/Conn
No longer blocks: b2g-RTSP-1.3
blocking-b2g: 1.3? → ---
Component: General → RTSP
Whiteboard: [ft:ril]
blocking-b2g: --- → backlog
Assignee: nobody → ettseng
Attached patch Reduce RtspTrackBuffer size (obsolete) — Splinter Review
Currently RtspTrackBuffer allocates unnecessary too large memory for video tracks:
  BUFFER_SLOT_NUM * BUFFER_SLOT_MAX_SIZE = 8KB * 8KB = 64MB

This patch reduces BUFFER_SLOT_NUM from 8192 to 512 and results in 4MB for one video track.
This value aligns to the default buffer size of media cache.

It might be a good idea to add a preference for setting this buffer size; however, there is a plan to refactor RtspTrackBuffer to use MediaCache, I suggest we just lower the total memory allocated for RTSP for now.

Benjamin, would this change have any impact on the solution of bug 990908?
Flags: needinfo?(bechen)
Status: NEW → ASSIGNED
(In reply to Ethan Tseng [:ethan] from comment #2)
> Created attachment 8407504 [details] [diff] [review]
> Reduce RtspTrackBuffer size
> 
> Currently RtspTrackBuffer allocates unnecessary too large memory for video
> tracks:
>   BUFFER_SLOT_NUM * BUFFER_SLOT_MAX_SIZE = 8KB * 8KB = 64MB
> 
> This patch reduces BUFFER_SLOT_NUM from 8192 to 512 and results in 4MB for
> one video track.
> This value aligns to the default buffer size of media cache.
> 
> It might be a good idea to add a preference for setting this buffer size;
> however, there is a plan to refactor RtspTrackBuffer to use MediaCache, I
> suggest we just lower the total memory allocated for RTSP for now.
> 
> Benjamin, would this change have any impact on the solution of bug 990908?
No, they are two independent bugs.
Flags: needinfo?(bechen)
Comment on attachment 8407504 [details] [diff] [review]
Reduce RtspTrackBuffer size

Hi Steve,
It's a minor change. Could you help to review this patch?
Attachment #8407504 - Flags: review?(sworkman)
Comment on attachment 8407504 [details] [diff] [review]
Reduce RtspTrackBuffer size

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

Patch looks fine. Performance is unaffected, right? What is the timeline for switching to MediaCache?
Attachment #8407504 - Flags: review?(sworkman) → review+
(In reply to Steve Workman [:sworkman] from comment #5)
> Patch looks fine. Performance is unaffected, right? What is the timeline for
> switching to MediaCache?
Yes. I've tested with many different cases, e.g. video, audio, play/pause/seek, different streaming sources and servers, ..., etc. The performance is unaffected (even better due to less time to allocate memory).
As I know, switching to MediaCache is still under research  and there is no specific timeline for it.
However it will quite possibly come true in the long run.
Whiteboard: [p=1]
Target Milestone: --- → 1.4 S6 (25apr)
Modify the title to reflect the real change.
Summary: Configure memory usage for RTSP streaming → Reduce memory usage for RTSP streaming
Just update the commit message.
Attachment #8407504 - Attachment is obsolete: true
Attachment #8407894 - Flags: review+
Update the result of TBPL (of the actual checked-in patch):
https://tbpl.mozilla.org/?tree=Try&rev=20adee90d418
https://hg.mozilla.org/mozilla-central/rev/234646463e96
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Verified the issue is fixed on 2.0 Flame (Master on 4/17/2014)

Memory usage is reduced for RTSP streaming
Verified as per TC#12922 in Comment 13
After comparing the browser usage before and after launching an RTSP video, the usage didn't increase more than 4mb

"Flame 2.0

Device: Flame 2.0 (319mb) (KitKat Base)(Full Flash)
Build ID: 20141212000203
Gaia: f3b9806f687fbbd7eba6b0e1f6ebb8bde09840ea
Gecko: 32f9fae79b89
Gonk: 263b5f41f7733c5577fb101eb4dc8ac5c11cfa8d
Version: 32.0 (2.0)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0"
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.