898866 - RTSP: seek function issues

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RTSP, defect)

Description

[Benjamin Chen [:bechen]]

STR:
1. play a RTSP video
2. seek to anywhere (especially backward seek)

result:
Sometimes the screen shows black screen.

Now we are guessing it is the packet timestamp issue to the decoder and RTSP server.
ex: seek to 15s by ui control panel.
when we are performing a seek function. we set the timestamp 15s into decoder.
But the timestamp of real packet comes from RTSP server is usually smaller than 15s.
After the seek operation, the packet data subsequently fill into RtspTrackBuffer but the decoder seems stop working or only read a few of packets.

I'll try to dig into MediaDecodeStateMachine to see what happened.

Comment 1

[Benjamin Chen [:bechen]]

See bug 877461 comment 49.

The bug is easily reproduce here because the RTSP doesn't buffer any data while seeking.
The seek calls nsBuiltinDecoderReader::DecodeToTarget to the desired point. The function OmxDecoder::ReadVideo spends about 5 seconds to get a "stale" frame or a normal frame.  The stale frame means that it has an incorrect timestamp. 
ex: assume 1 frame per second , now we perform seek from 30s to 10s.
The timestamp of normal case:
10s, 11s, 12s ...
The timestamp of error case:
30s, 10s, 11s, 12s ...
10s, 11s, 12s, 30s, 13s

Note that there are 2 or more error cases.
The stale frame set the |remainingTime| in MediaDecoderStateMachine::AdvanceFrame() into a very large number (30 - 10) that causes the decoder stop working.

Comment 2

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Does this block bug 929372?

Comment 4

[Jason Smith [:jsmith]]

RTSP is not a committed feature for 1.3, so this should not block the release.

Comment 5

[Bruce Sun [:brsun]]

Attachment: bug898866_rtsp_seek_fails.patch

Hi Benjamin,

I've tried to modify the timing of calling nsIStreamingProtocolController::Pause() and nsIStreamingProtocolController::Play() to sync with decoding thread life cycle.
Would you please help check if this can ease the deadlock which is caused by network data starvation?

Thanks in advance.

Comment 6

[Preeti Raghunath(:Preeti)]

Has RTSP landed in 1.3. This was not a committed feature so we will not be blocking on the bug if feature has not landed.

Please confirm.

Comment 7

[Vincent Chang[:vchang][changyihsin]]

(In reply to Bruce Sun [:brsun] from comment #5)
> Created attachment 8342959 [details] [diff] [review]
> bug898866_rtsp_seek_fails.patch
> 
> Hi Benjamin,
> 
> I've tried to modify the timing of calling
> nsIStreamingProtocolController::Pause() and
> nsIStreamingProtocolController::Play() to sync with decoding thread life
> cycle.
> Would you please help check if this can ease the deadlock which is caused by
> network data starvation?
> 
> Thanks in advance.
I have verified this patch and the result looks really good. Good job :-)
Your patch seems fixed the decoder thread deadlock problem, can we post your patch to Bug 947113 ?

Comment 8

[Bruce Sun [:brsun]]

(In reply to Vincent Chang[:vchang] from comment #7)
> I have verified this patch and the result looks really good. Good job :-)
> Your patch seems fixed the decoder thread deadlock problem, can we post your
> patch to Bug 947113 ?

Sure. I will upload my patch to bug 947113 after refining my comments.

Comment 9

[Vincent Chang[:vchang][changyihsin]]

The timestamp issue bechen found here is not reproducible on unagi right now after apply the patches in bug 947101 and bug 947113. But we would like to leave the bug open for a while to track it.

Comment 10

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

RTSP defect to be fixed in 1.3.

Comment 11

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Is it blocked by bug 947114, which is no longer 1.3+?

Comment 12

[Benjamin Chen [:bechen]]

bug 947114 would not block this anymore, because it won't let the device crash or stuck.
We keep the bug 947114 open for tracking.

Comment 13

[Sri Kasetti]

RTSP video streaming is not 1.3 blocker. Please change the status if you disagree...

Comment 14

[Preeti Raghunath(:Preeti)]

Wesley

Is this function common to both audio and video?

Comment 15

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Seek function is common, especially in video case IMO (think about YouTube).
However, this bug is tracking bug for "seek" related issues so shouldn't block.(please refer to "depends on" bugs; critical ones are resolved already.)
Titled renamed to avoid confusing.

Comment 16

[Jason Smith [:jsmith]]

Ethan - As you know from offline, we've decided we will block on any feature blocker for video RTSP support. Can you nominate any of the dependencies of this bug for 1.3? that you think must be fixed to ensure that seeking works correctly with video RTSP URLs, since we can't block on this bug directly (as it's a tracking bug)?

Comment 17

[Ethan Tseng [:ethan]]

Hi Jason, I checked the bug list related to RTSP seeking function. I think they were all already specified in the "Depends on" field of this bug.

Currently there are two open bugs on which this bug depends:
Bug 961926 - [RTSP] Seek function does not work for certain sources (New)
Bug 947114 - Rtsp: there is noise in the picture after perform seek (New) (not blocker)

Bug 947114 was already identified as not a blocker.
We just need to determine whether bug 961926 should be nominate to 1.3 blocker or not.

Comment 18

[Jason Smith [:jsmith]]

(In reply to Ethan Tseng [:ethan] from comment #17)
> Hi Jason, I checked the bug list related to RTSP seeking function. I think
> they were all already specified in the "Depends on" field of this bug.
> 
> Currently there are two open bugs on which this bug depends:
> Bug 961926 - [RTSP] Seek function does not work for certain sources (New)
> Bug 947114 - Rtsp: there is noise in the picture after perform seek (New)
> (not blocker)
> 
> Bug 947114 was already identified as not a blocker.
> We just need to determine whether bug 961926 should be nominate to 1.3
> blocker or not.

Agreed bug 947114 won't block. I do think bug 961926 however does, as that implies seeking with 3GP video files in a RTSP video stream isn't working.

Comment 19

[Ken Chang[:kenkai]]

It blocks 1.4 feature.

Comment 20

[Kai-Chih Hu [:khu]]

(In reply to Ken Chang[:ken] from comment #19)
> It blocks 1.4 feature.

So, does it match any one of the criteria to be a blocker: 
https://wiki.mozilla.org/B2G/Triage#Issues_that_Should_Block

Comment 21

[Jason Smith [:jsmith]]

This doesn't fall under the QC blocking feature list & DSDS feature list. Renoming.

Comment 22

[Ken Chang[:kenkai]]

No more 1.4+ feature now.

Comment 23

[Ken Chang[:kenkai]]

It is not a 1.4 feature.

Comment 24

[Ethan Tseng [:ethan]]

All the blockers of this bug were resolved.
The basic seek function of RTSP is assured to be workable right now.

Set this bug as resolved fixed as well.

Comment 25

[William Hsu [:whsu]]

Good Job, Ethan! \^O^/~