Frame drops observed during RTSP streaming

RESOLVED FIXED in Firefox 35, Firefox OS v2.1

Status

Firefox OS
RTSP
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bhargavg1, Assigned: ethan)

Tracking

unspecified
2.1 S6 (10oct)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1+, firefox33 wontfix, firefox34 wontfix, firefox35 fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

(Whiteboard: [caf priority: p2][CR 710872] [p=15])

Attachments

(7 attachments, 11 obsolete attachments)

202.13 KB, image/png
Details
410.67 KB, image/png
Details
4.82 KB, patch
Details | Diff | Splinter Review
7.75 KB, patch
Details | Diff | Splinter Review
612 bytes, text/plain
Details
18.45 KB, patch
Details | Diff | Splinter Review
16.29 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

4 years ago
+++ This bug was initially created as a clone of Bug #1050351 +++

Compared to other platforms (like LA) see lot of frame drops while doing RTSP streaming on FFOS. 

Do we have any competitive comparison on the overall quality for RTSP streaming compared to other platforms
(Reporter)

Updated

4 years ago
No longer blocks: 1025317
No longer depends on: 1050351
(Reporter)

Comment 1

4 years ago
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.0?
(Assignee)

Comment 2

4 years ago
(In reply to bhargavg1 from comment #0)
> Compared to other platforms (like LA) see lot of frame drops while doing
> RTSP streaming on FFOS. 
Excuse me. What does "LA" stand for?

Comment 3

4 years ago
LA - Linux Android. Sorry for not being clear, this is an internal term we use for Android.
(Assignee)

Comment 4

4 years ago
(In reply to Inder from comment #3)
> LA - Linux Android. Sorry for not being clear, this is an internal term we
> use for Android.
Thanks, Inder.
(Assignee)

Comment 5

4 years ago
(In reply to bhargavg1 from comment #0)
> Do we have any competitive comparison on the overall quality for RTSP
> streaming compared to other platforms

Yes. Mostly we compare our RTSP streaming to Android and sometimes to VLC player.
We had setup Darwin Streaming Server ourselves and compare the clients' performance in our LAN.
We had also compare their performance by some public RTSP links.

Could you describe more specifically about the frame drop condition you encounter?
1) Do you see many broken blocks (in green or grey color) in the video? Or the playback is not smooth?
2) What kind of media codecs do you observe frame drops?
In our experience, the performance could be quite variant for different codecs. While many other factors could also impact the client's performance.
It would be helpful if we have more clues. :)
(Assignee)

Updated

4 years ago
Assignee: nobody → ettseng
(Assignee)

Comment 6

4 years ago
Excuse me. I have several more questions here.
1. Did you compare FFOS and LA using the same device or chipset?
2. Do you use any tool to observe/analyze frame drops?
   If so, what is the frame drop rate you observed?

Comment 7

4 years ago
Hi Inder & Bhargavg, can you provide feedback to Comment 5 and Comment 6? Thanks.
Flags: needinfo?(ikumar)
Flags: needinfo?(bhargavg1)
(Reporter)

Comment 8

4 years ago
(In reply to Ethan Tseng [:ethan] from comment #5)
> (In reply to bhargavg1 from comment #0) 

Info from our test teams,

> Could you describe more specifically about the frame drop condition you
> encounter?
> 1) Do you see many broken blocks (in green or grey color) in the video? Or
> the playback is not smooth?
Even though the playback looks smooth, we see framedrops in the logs. We did see cases where the playback pauses sometimes and expect it to be because of buffering and broken blocks

> 2) What kind of media codecs do you observe frame drops?
We are seeing frame drops for all the codecs( H264, H263 & MPEG4)

> 3)Did you compare FFOS and LA using the same device or chipset?
This info is when compared against the same chipset to conclude if the issue is there on the base platform or not

> 4) Do you use any tool to observe/analyze frame drops?
Purely rely on the adb logs to see the frame drops. Ideally the frame drops shouldn’t be greater than 1% ( Total dropped frames/Total decoded frames) during End to End playback and 3%during seek. We are seeing frame drops beyond this level.

We rely on shared/js/media/video_stats.js on video element to find the total dropped frames and decoded frames.
Flags: needinfo?(ikumar)
Flags: needinfo?(bhargavg1)

Comment 9

4 years ago
Hi Ethan, can you try shared/js/media/video_stats.js, this path is under Gaia repository.
Flags: needinfo?(ettseng)
(Assignee)

Comment 10

4 years ago
(In reply to howie [:howie] from comment #9)
> Hi Ethan, can you try shared/js/media/video_stats.js, this path is under
> Gaia repository.

Thanks for information! I'll try it.
Flags: needinfo?(ettseng)
(Assignee)

Comment 11

4 years ago
(In reply to bhargavg1 from comment #8)
> > 2) What kind of media codecs do you observe frame drops?
> We are seeing frame drops for all the codecs( H264, H263 & MPEG4)
Hm... It seems all codecs suffer.
I will try to observe the frame drop rate in the way you did.

> > 4) Do you use any tool to observe/analyze frame drops?
> Purely rely on the adb logs to see the frame drops. Ideally the frame drops
> shouldn’t be greater than 1% ( Total dropped frames/Total decoded frames)
> during End to End playback and 3%during seek. We are seeing frame drops
> beyond this level. 
> We rely on shared/js/media/video_stats.js on video element to find the total
> dropped frames and decoded frames.
Could you share how did you use the tool "video_stats.js"?
I see videoStats.start() requires an HTMLVideoElement as parameter.
So did you write an app on your own and create HTMLVideoElement in your JS?
Or you still play RTSP by the browser app? If so, how did you utilize video_stat.js?
Flags: needinfo?(bhargavg1)
(Reporter)

Comment 12

4 years ago
(In reply to Ethan Tseng [:ethan] from comment #11)
> (In reply to bhargavg1 from comment #8)
> > > 4) Do you use any tool to observe/analyze frame drops?
> > Purely rely on the adb logs to see the frame drops. Ideally the frame drops
> > shouldn’t be greater than 1% ( Total dropped frames/Total decoded frames)
> > during End to End playback and 3%during seek. We are seeing frame drops
> > beyond this level. 
> > We rely on shared/js/media/video_stats.js on video element to find the total
> > dropped frames and decoded frames.
> Could you share how did you use the tool "video_stats.js"?
> I see videoStats.start() requires an HTMLVideoElement as parameter.
> So did you write an app on your own and create HTMLVideoElement in your JS?
> Or you still play RTSP by the browser app? If so, how did you utilize
> video_stat.js?

we have a small internal app forked off the mainline video app to take an
option to play either a local file or an URL (http/rtsp streams). This app uses the video_stats.js funcs to pull in the necessary info
Flags: needinfo?(bhargavg1)
(Assignee)

Comment 13

4 years ago
(In reply to bhargavg1 from comment #12)
> we have a small internal app forked off the mainline video app to take an
> option to play either a local file or an URL (http/rtsp streams). This app
> uses the video_stats.js funcs to pull in the necessary info

I got it. Thanks for your information.
I'll try to observer dropped frame rate on our side.
(In reply to Ethan Tseng [:ethan] from comment #13)
> (In reply to bhargavg1 from comment #12)
> > we have a small internal app forked off the mainline video app to take an
> > option to play either a local file or an URL (http/rtsp streams). This app
> > uses the video_stats.js funcs to pull in the necessary info
> 
> I got it. Thanks for your information.
> I'll try to observer dropped frame rate on our side.

Blocking you could observe the frame drop, if this should not be blocking, please re-nom.
blocking-b2g: 2.0? → 2.0+
(Assignee)

Comment 15

4 years ago
Created attachment 8481227 [details]
RTP Receiver Block Diagram.png

A block diagram of an RTP receiver.
(Assignee)

Comment 16

4 years ago
I plan to do two tasks to analyze the root cause of frame drop.

1) Observe the frame drop rate using video_stats.js.
   Purpose: To reproduce the reported issue, that is, total frame drop rate > 1%.
   How    : Launch video app for RTSP links (we did this before in v1.3 and v1.4).
   Comment: video_stats.js uses the Web API getVideoPlaybackQuality() to retrieve frame statistics.

2) Observe the counter of input and output access-units of RTP assembler.
   Purpose: To exclude RTP assembler being the factor of frame drop.
   How    : Add logs in the input and output of XXXAssembler.
   Comment: If we see frame drops from video_stats.js but no drop in RTP assembler,
            we can be sure the performance bottleneck is not in RTP assembler.
            It could be HW decoder or other components.

Refer to the diagram in attachment 8481227 [details].
(Assignee)

Comment 17

4 years ago
Created attachment 8482199 [details]
How does VideoPlaybackQuality calcuate counters

A block diagram illustrating how the VideoPlaybackQuality calculates the counters of video statistics.
(Assignee)

Comment 18

4 years ago
See attachment 8482199 [details].

The VideoPlaybackQuality interface provides three counters about video frames.
They are calculated in HTMLVideoElement::GetVideoPlaybackQuality():
* totalVideoFrames     = parsedFrames
* droppedVideoFrames   = totalVideoFrames - presentedFrames
* corruptedVideoFrames = totalVideoFrames - decodedFrames

"corruptedVideoFrames" is caused by HW decoder.
Another counter "droppedByAVSync", which is not shown by VideoPlaybackQuality, is caused by
AV Sync Buffer (in MediaDecoderStateMachine)

They both contribute to "droppedVideoFrames".
(Assignee)

Comment 19

4 years ago
Benjamin, please provide your input to comment 18, thanks!
Flags: needinfo?(bechen)
(Assignee)

Comment 20

4 years ago
(In reply to Ethan Tseng [:ethan] from comment #16)
> 1) Observe the frame drop rate using video_stats.js.
I used a patch to launch the video app for RTSP scheme.
We do see a significant frame drop rate reporting from video_stats.js.
The formula is listed in comment 18.

> 2) Observe the counter of input and output access-units of RTP assembler.
>    Purpose: To exclude RTP assembler being the factor of frame drop.
This step is not required since we already observe frame drops in step 1.
(In reply to Ethan Tseng [:ethan] from comment #18)
> See attachment 8482199 [details].
> 
> The VideoPlaybackQuality interface provides three counters about video
> frames.
> They are calculated in HTMLVideoElement::GetVideoPlaybackQuality():
> * totalVideoFrames     = parsedFrames
> * droppedVideoFrames   = totalVideoFrames - presentedFrames
> * corruptedVideoFrames = totalVideoFrames - decodedFrames
> 
> "corruptedVideoFrames" is caused by HW decoder.
> Another counter "droppedByAVSync", which is not shown by
> VideoPlaybackQuality, is caused by
> AV Sync Buffer (in MediaDecoderStateMachine)
> 
> They both contribute to "droppedVideoFrames".

There are two kinds of frame drops:
1. corruptedVideoFrames:
When our statemachine feels the decoding speed is too slow, it will trigger "skip to next I-frame", but unfortunately, our RTSP doesn't implement this kind of mechanism. That make the number of corruptedVideoFrames huge.
http://dxr.mozilla.org/mozilla-central/source/content/media/MediaDecoderStateMachine.cpp#612

For example: statemachine wants the frame after 5s, but we still input the 4s, 4.5s 4.8s data into HW decoder, these 3 frames are treated as corruptedVideoFrames.
If we really want to solve this, we can do at Bug 897486.

2. droppedByAVSync:
This seems an systematic problem, we need more analysis on it.
Ex: If there are 1s 2s 3s three video frames in video queue, but now the audio clock had played at 4s, the 1s 2s frames are treated as droppedByAVSync frames. Only the 3s frame will be displayed.
Means we need a faster HW video decoder or we need go into "buffer state".

In Overall, I guess our HW might waste some decoding power for the corruptedVideoFrames, make the droppedByAVSync worst.
Flags: needinfo?(bechen)
(Assignee)

Comment 22

4 years ago
(In reply to Benjamin Chen [:bechen] from comment #21)
> In Overall, I guess our HW might waste some decoding power for the
> corruptedVideoFrames, make the droppedByAVSync worst.
Benjamin, thanks for your input!
As discussed offline today, we can trace the problem starting here:
http://dxr.mozilla.org/mozilla-central/source/content/media/MediaDecoderStateMachine.cpp#612

Compared to HTTP streaming, RTSP streaming enters this block quite often.
I'll look into those variables to find out why.
Also, we need to check the network status here. If the network speed is lower than HW decoder, the thread will be block here:
http://dxr.mozilla.org/mozilla-central/source/content/media/RtspMediaResource.cpp?from=RtspMediaResource.cpp#222

For HTTP streaming:
http://dxr.mozilla.org/mozilla-central/source/content/media/MediaCache.cpp?from=MediaCache.cpp#2231
Created attachment 8483336 [details] [diff] [review]
WIP-rtsp_byteconsume.patch

wip patch to testing.
(Assignee)

Comment 25

4 years ago
*** Progress Update ***
We made some measurement and experiments but are still working to identify the root cause.

Observations:
1. Frame drop rate between HTTP and RTSP streaming.
   Server   : Apache server and DarwinStreamingServer on the same PC (Ubuntu Linux)
   Video    : H264, AAC, 720p, 30fps (unhinted for HTTP and hinted for RTSP)
   Duration : 45 sec
   Player   : Gaia video app (Flame v2.1)
   Statistic: VideoPlaybackQuality from video_stats.js
                   TotalFrames    DroppedFrames    DroppedFrameRate
   HTTP streaming  around 1350    1~2              < 0.15%
   RTSP streaming  around 1350    300~400          22~30%

2. Since HTTP and RTSP streaming use the same HW decoder and the same MediaDecoderStateMachine,
   we believe the video frame drops of RTSP are possibly caused by the design of playout buffer,
   which is mainly implemented in RtspMediaResource and RtspTrackBuffer.
   (Playout buffer is illustrated in attachment 8482199 [details])

3. One important purpose of the playout buffer is to compensate for network timing variability.
   We'll further analyze our playout buffer satisfy this feature or not.

4. We measured RTSP quality by playing low resolution video, such as 144p.
   The frame drop rate drops to 3~5%.
   By reviewing the current implementation, memory copies of video frames could be a burden to
   the playback performance.
(Reporter)

Comment 26

4 years ago
(In reply to Ethan Tseng [:ethan] from comment #20)
> I used a patch to launch the video app for RTSP scheme.

Ethan, can you point to the patch as to how we can modify the video app to take an RTSP stream URL

Comment 27

4 years ago
Created attachment 8484724 [details] [diff] [review]
log.patch

I added some code to log the time of incoming frames. I found that
1. sometimes the parent process sends lots of frames to content process but content process gets the frames after almost 1 second
2. when I boot b2g device and do nothing, b2g process uses about 20% CPU. I will spend time on profiling it.

Comment 28

4 years ago
The high CPU problem is filed on bug 1062119.
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1055936
(Reporter)

Updated

4 years ago
Blocks: 1041241
Whiteboard: [CR 710872]

Updated

4 years ago
Whiteboard: [CR 710872] → [caf priority: p2][CR 710872]
(Assignee)

Comment 30

4 years ago
Created attachment 8484851 [details] [diff] [review]
Launch Video App for RTSP (Gecko part)

This patch basically reverts the code changes of bug 1000340.
(Assignee)

Comment 31

4 years ago
Created attachment 8484853 [details]
Launch Video App for RTSP (Gaia part)

How to launch video app for RTSP scheme.
(Assignee)

Comment 32

4 years ago
(In reply to bhargavg1 from comment #26)
> Ethan, can you point to the patch as to how we can modify the video app to
> take an RTSP stream URL

You can launch the video app for RTSP scheme by these steps:
1. Apply the patch of attachment 8484851 [details] [diff] [review].
2. Follow the instructions in attachment 8484853 [details] to modify your video app.
3. Flash gecko and install gaia.
4. Open the browser app. Clicking any RTSP URL would launch the video app now.

If you have any problem, please let me know. Thanks.
(Assignee)

Comment 33

4 years ago
Created attachment 8484885 [details] [diff] [review]
WIP to fix frame drop of RTSP

Add a fixed playout delay (2 sec) to reduce frame drop rate.
We see the frame drop rate become less than 2% by applying this patch.

Hi Bhargavg,
Please note this is just a experimental patch, not a final solution.
If you have time, please help to test with this patch and let us know
your result. Thanks!
Flags: needinfo?(bhargavg1)
(Assignee)

Comment 34

4 years ago
A meta bug 1063466 was filed to track all the issues we discovered while investigating this bug.
We plan to focus on fixing the "playout buffer delay", which is the top bottleneck for now, in this bug.
(Reporter)

Comment 35

4 years ago
(In reply to Ethan Tseng [:ethan] from comment #33)
> Created attachment 8484885 [details] [diff] [review]
> WIP to fix frame drop of RTSP
> 
> Hi Bhargavg,
> Please note this is just a experimental patch, not a final solution.
> If you have time, please help to test with this patch and let us know
> your result. Thanks!

Sure will try this out hopefully should have something by Monday

Comment 36

4 years ago
According to comment 34, I am thinking that having *playout buffer delay* is a big feature. I wonder if we can remove it from 2.0 CAF and add it in 2.1.
Flags: needinfo?(bbajaj)
(Reporter)

Comment 37

4 years ago
(In reply to Ethan Tseng [:ethan] from comment #33)
> Created attachment 8484885 [details] [diff] [review]
> WIP to fix frame drop of RTSP
> 

Results from testing the patch. 

Listed both without and with patch results 

Clip                  frame-drops-WIP     frame-drops-without-patch
clip1_1080p.mp4           195/1204            196/1209 
clip2_720p.mp4            151/1040            102/1052
Flags: needinfo?(bhargavg1)

Comment 38

4 years ago
[Blocking Requested - why for this release]:

Ken: based on offline discussion, moving this out of 2.0 to 2.1. We may want to backport to 2.0 in future with proper risk assessment in case customer requests.
Blocks: 1025317
No longer blocks: 1041241
blocking-b2g: 2.0+ → 2.1?
Flags: needinfo?(bbajaj)

Updated

4 years ago
Blocks: 1041241
(Reporter)

Updated

4 years ago
No longer blocks: 1041241

Updated

4 years ago
blocking-b2g: 2.1? → 2.1+
Comment on attachment 8487041 [details] [diff] [review]
Patch v1.0

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

1. The playout-delay should also apply to seek.
2. We should have a check for the occupancy of RtspTrackBuffer, and terminate the playout-delay when buffer is almost full.
3. The EOS/network error should also terminate the playout-delay.
4. Do we need to adjust the delay time dynamically?

::: content/media/RtspMediaResource.cpp
@@ +61,5 @@
>    , mSlotSize(aSlotSize)
>    , mTotalBufferSize(BUFFER_SLOT_NUM * mSlotSize)
>    , mFrameType(0)
> +  , mIsStarted(false)
> +  , mPlayoutDely(false) {

typo

@@ +389,5 @@
>    mMonitor.NotifyAll();
>  }
>  
> +void RtspTrackBuffer::FireTimerCallback(nsITimer *aTimer, void *aClosure) {
> +  RtspTrackBuffer *pRes = reinterpret_cast<RtspTrackBuffer *>(aClosure);

static_cast
Attachment #8487041 - Flags: feedback?(bechen)
(Assignee)

Comment 41

4 years ago
(In reply to Vincent Chang[:vchang] from comment #39)
> Created attachment 8487041 [details] [diff] [review]
> Patch v1.0
Thanks for providing this patch!
I will test and see how much improvement we can make from it.
Hi Benjamin, thanks for your feedback on the patch. Per offline talk with Ethan, he will provide the follow-up patch to address you comments.
(Assignee)

Comment 43

4 years ago
Created attachment 8490541 [details] [diff] [review]
framedrop-wip.patch

WIP patch: Based on Vincent's patch. Add playout delay mechanism, in order to compensate timing variation caused by network.
Attachment #8484885 - Attachment is obsolete: true

Comment 44

4 years ago
Created attachment 8491417 [details] [diff] [review]
0001-add-log.patch

This patch is to dump some time stamp when a "whole" video frame is received on
1. b2g onTrackDataAvailable
2. b2g send meta data to content process
3. content process receives the frame on RecvOnMediaDataAvailable
4. content process handles the frame on RtspMediaResource::OnMediaDataAvailable
5. content process writes the frame to buffer(WriteBuffer)
6. content process reads the frame from buffer(ReadBuffer)
7. content process decodes the frame 
8. content process render the frame
Attachment #8483336 - Attachment is obsolete: true
After apply the playout-delay and slee's patch, now we suspect the bottleneck is the |mDecodeTaskQueue| in the StateMachine.

The TaskQueue executes the "DecodeAudio" and "DecodeVideo" sequentially, once the "playout-delay buffer" is exhausted, the "DecodeAudio" might be blocked then the "DecodeVideo" is blocked too.
Created attachment 8495098 [details] [diff] [review]
playoutdelay-wip.patch
(Assignee)

Comment 47

4 years ago
Created attachment 8495201 [details] [diff] [review]
bug-1056187-part1.patch

WIP patch: Add 3-sec playout delay to RtspTrackBuffer.

Benjamin, this patch does reduce the frame drop rate, but the playback
is not smooth in the first few seconds.
From logs I can see several pause-play command pairs in the beginning.
Let's discuss this tomorrow.
Attachment #8490541 - Attachment is obsolete: true

Updated

4 years ago
Target Milestone: --- → 2.1 S6 (10oct)
See Also: → bug 1068963
(Assignee)

Comment 48

4 years ago
We filed two bugs to unravel the causes and solutions of this bug.
Depends on: 1075400, 1074791

Comment 49

4 years ago
Ethan, it seems that we can not have a fix for this bug before 10/7. Is it possible to fix it before 10/10?
(Assignee)

Comment 50

4 years ago
(In reply to Ken Chang[:ken] from comment #49)
> Ethan, it seems that we can not have a fix for this bug before 10/7. Is it
> possible to fix it before 10/10?

Bug 1075400 already got r+ and is in landing process now.
We also have an idea how to fix bug 1074791 and I'm working on the patch.
I think we can fix bug 1074791 in 2-3 days, that is, before 10/10.
I'll provide a patch for this bug soon.

In conclusion, it's possible to fix the whole frame drop issue by 10/10.
I'll keep updating the status.
(Assignee)

Comment 51

4 years ago
Created attachment 8500910 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

Hi Benjamin,
Please help to review this patch. :)
Attachment #8487041 - Attachment is obsolete: true
Attachment #8495098 - Attachment is obsolete: true
Attachment #8495201 - Attachment is obsolete: true
Attachment #8500910 - Flags: review?(bechen)
Comment on attachment 8500910 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

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

Overall looks good to me. Please add comments as possible as you can.

::: content/media/RtspMediaResource.cpp
@@ +461,5 @@
> +
> +// static
> +void
> +RtspTrackBuffer::PlayoutDelayTimerCallback(nsITimer *aTimer,
> +                                           void *aClosure)

Since the aClosure is a raw pointer, we need to cancel this timer before ~RtspTrackBuffer() .

::: content/media/omx/RtspOmxReader.cpp
@@ +91,5 @@
> +
> +  // FIXME: Calling SetIdle() impacts performance, but not calling it might be
> +  // problematic when auto-play is disabled.
> +  // Stop the network traffic because the stream is not really played.
> +  // SetIdle();

If we don't call SetIdle() function here, the network data won't stop if we are go into "waiting resource state".

@@ +94,5 @@
> +  // Stop the network traffic because the stream is not really played.
> +  // SetIdle();
> +  if (rv == NS_OK && !IsWaitingMediaResources()) {
> +    mRtspResource->EnablePlayoutDelay();
> +  }

Call SetIdle() function if we are waiting resources.
//
else if (IsWaitingMediaResources  ) {
SetIdle();
}
Attachment #8500910 - Flags: review?(bechen) → review+
(Assignee)

Comment 53

4 years ago
Comment on attachment 8500910 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

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

::: content/media/RtspMediaResource.cpp
@@ +461,5 @@
> +
> +// static
> +void
> +RtspTrackBuffer::PlayoutDelayTimerCallback(nsITimer *aTimer,
> +                                           void *aClosure)

The timer is canceled by StopPlayoutDelay().
And StopPlayoutDelay() is called in RtspTrackBuffer::Stop() and Reset().
Is there any other case we should take care of?

::: content/media/omx/RtspOmxReader.cpp
@@ +94,5 @@
> +  // Stop the network traffic because the stream is not really played.
> +  // SetIdle();
> +  if (rv == NS_OK && !IsWaitingMediaResources()) {
> +    mRtspResource->EnablePlayoutDelay();
> +  }

Okay, I'll add this.
(Assignee)

Comment 54

4 years ago
Created attachment 8501610 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

Addresses issues in comment 52.
Attachment #8500910 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
Whiteboard: [caf priority: p2][CR 710872] → [caf priority: p2][CR 710872] [p=15]
(Assignee)

Comment 55

4 years ago
Created attachment 8501637 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

1. Refresh comment "r=bechen".
2. Fix a compile warning of comparison between signed and unsigned integer expressions
Attachment #8501610 - Attachment is obsolete: true
(Assignee)

Comment 56

4 years ago
Created attachment 8501638 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

Truncate white spaces.
Attachment #8501637 - Attachment is obsolete: true
(Assignee)

Comment 57

4 years ago
After applying the fixes of bug 1075400, bug 1074791 and this bug, the frame drop rate could be less 
than 1%.

The video statistics dumped on my Flame 2.2 today:

I/GeckoDump(31210): Video statistics start
I/GeckoDump(31210): Dimensions: 1280x720
I/GeckoDump(31210): Complete duration: 00:45
I/GeckoDump(31210): Start time: 00:00
I/GeckoDump(31210): End time: 00:00
I/GeckoDump(31210): Total frames: 1340
I/GeckoDump(31210): Decoded frames: 1340
I/GeckoDump(31210): Corrupted frames: 0
I/GeckoDump(31210): Rendered frames: 1339
I/GeckoDump(31210): Dropped frames: 1
I/GeckoDump(31210): Average rendered FPS: 28.45
(Assignee)

Comment 58

4 years ago
Created attachment 8501841 [details] [diff] [review]
bug-1056187-fix.patch

Fixed compile error revealed by Treeherder.
Attachment #8501638 - Attachment is obsolete: true
(Assignee)

Comment 59

4 years ago
Created attachment 8501847 [details] [diff] [review]
Add playout delay to RtspTrackBuffer
Attachment #8501841 - Attachment is obsolete: true
(Assignee)

Comment 60

4 years ago
The result of Treeherder:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8e4d441013bd

Request check-in of the patch of attachment 8501847 [details] [diff] [review].
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c5a41dac5729
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Please request b2g34 approval on this patch when you get a chance.
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → fixed
status-firefox33: --- → wontfix
status-firefox34: --- → wontfix
status-firefox35: --- → fixed
Flags: needinfo?(ettseng)
(Assignee)

Comment 64

4 years ago
Comment on attachment 8501847 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

Approval Request Comment
[Feature/regressing bug #]: Reduce frame drop rate over RTSP
[User impact if declined]: RTSP streaming playback is not smooth
[Describe test coverage new/current, TBPL]: On m-c
[Risks and why]: None
[String/UUID change made/needed]: None
Attachment #8501847 - Flags: approval-mozilla-b2g32?
Flags: needinfo?(ettseng)
(Assignee)

Comment 65

4 years ago
Comment on attachment 8501847 [details] [diff] [review]
Add playout delay to RtspTrackBuffer

Correct the approval flag.
Attachment #8501847 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g34?
(Assignee)

Comment 66

4 years ago
Hi Ryan,
We intend to land this patch to v2.1.
Should I also request aurora approval on the patch?
Flags: needinfo?(ryanvm)
v2.1 is moving to b2g34 on Monday :)
Flags: needinfo?(ryanvm)
(Assignee)

Comment 68

4 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #67)
> v2.1 is moving to b2g34 on Monday :)
Got it. Thanks!
(Assignee)

Comment 69

4 years ago
(In reply to Ethan Tseng [:ethan] from comment #57)
> After applying the fixes of bug 1075400, bug 1074791 and this bug, the frame
> drop rate could be less than 1%.

Please notice that although the frame drop rate is low after these 3 fixes, we can still see a obvious
lagging near the beginning of playback (3~4 sec).
The root cause was found and fixed in bug 1080470.
Depends on: 1080470

Updated

4 years ago
Attachment #8501847 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
(Assignee)

Comment 71

4 years ago
Bhargavg,

Please note our solution made a tradeoff between quality and promptness.
After the fix, we have to wait for around 3 seconds in the initial playback.
The media player is buffering enough stream data for smoothing playback during this period.

We think this behavior is reasonable and acceptable for streaming.
Android phone has the same buffering behavior (it waits for 4~5 sec).

Comment 72

4 years ago
HI Ethan,
 We have tried again with the fix and observed that frame drop of H264 test clips is reduced to 1% but we are still seeing huge frame drop for MPEG4 test clip.

Have sent an email with attached MPEG4 test clip where we are seeing huge frame drop and also while RTSP playback video stops and audio continues.

Video Stats:
10-28 18:15:21.516  1516  1516 I GeckoDump: Total frames: 140
10-28 18:15:21.516  1516  1516 I GeckoDump: Decoded frames: 66
10-28 18:15:21.516  1516  1516 I GeckoDump: Corrupted frames: 74
10-28 18:15:21.516  1516  1516 I GeckoDump: Rendered frames: 56
10-28 18:15:21.516  1516  1516 I GeckoDump: Dropped frames: 84
10-28 18:15:21.516  1516  1516 I GeckoDump: Average rendered FPS: 0.82
Flags: needinfo?(ettseng)

Comment 73

4 years ago
HI Ethan,

As you are already working on this (as per your mail) I am re-opening this bug, would be easy to track.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 74

4 years ago
(In reply to Pooja from comment #73)
> HI Ethan,
> As you are already working on this (as per your mail) I am re-opening this
> bug, would be easy to track.

Hi Pooja,

Sorry for my late response.

I hinted the clip and played it over RTSP streaming by different players.
1) FXOS (Flame v2.2)    : Most video frames cannot be rendered, the screen is frozen most of the time.
                          Audio frames are played but quality is poor.
2) VLC (desktop v2.1.5) : Better than FXOS. But the overall quality is still poor.
3) Android (Nexus5 v4.4): Better than FXOS. But the overall quality is still poor.

It seems the MPEG4 format in this clip is not well supported by most streaming players.
Based on this result, I doubt whether this clip should be the quality criteria for us.
Flags: needinfo?(ettseng)
(Assignee)

Comment 75

4 years ago
In my opinion, this is kind of a format issue but not performance issue.
I suggest we file a new bug to trace it.
(Assignee)

Comment 76

4 years ago
I filed bug 1093548 - [RTSP] Poor quality for certain MPEG4-AAC clip.

Comment 77

4 years ago
Hi Pooja, do you have any feedback for comment 74 ?
Flags: needinfo?(poojas)

Comment 78

4 years ago
Agree with comment #74.
Because we are not seeing same huge frame drop for all 720p/1080p clips but only with some specific clips.
Flags: needinfo?(poojas)
(Assignee)

Comment 79

4 years ago
(In reply to Pooja from comment #78)
> Agree with comment #74.
> Because we are not seeing same huge frame drop for all 720p/1080p clips but
> only with some specific clips.

Okay. Then I'll set this bug as resolved again.
Let's trace the specific clip issue in bug 1093548.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Unable to verify this bug and dependencies as it is a back-end issue.
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
(In reply to Yeojin Chung [:YeojinC] from comment #80)
> Unable to verify this bug and dependencies as it is a back-end issue.

Correction: We do not have the necessary tools to verify this issue.
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [QAnalyst-Triage+][QAnalyst-verify-]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.