RTSP: seek function issues

RESOLVED FIXED in 2.0 S2 (23may)

Status

Firefox OS
RTSP
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: bechen, Assigned: bechen)

Tracking

(Blocks: 1 bug)

unspecified
2.0 S2 (23may)
x86_64
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(tracking-b2g:backlog)

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 892360
(Assignee)

Comment 1

5 years ago
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.
Does this block bug 929372?
blocking-b2g: --- → 1.3?
Whiteboard: [ft:ril]

Updated

4 years ago
Duplicate of this bug: 933194
blocking-b2g: 1.3? → 1.3+
Target Milestone: --- → 1.3 Sprint 6 - 12/6
RTSP is not a committed feature for 1.3, so this should not block the release.
blocking-b2g: 1.3+ → 1.3?

Comment 5

4 years ago
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.
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.
(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

4 years ago
(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.
Target Milestone: 1.3 Sprint 6 - 12/6 → ---
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.
RTSP defect to be fixed in 1.3.
blocking-b2g: 1.3? → 1.3+
Is it blocked by bug 947114, which is no longer 1.3+?
Flags: needinfo?(bechen)
(Assignee)

Comment 12

4 years ago
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.
Flags: needinfo?(bechen)

Comment 13

4 years ago
RTSP video streaming is not 1.3 blocker. Please change the status if you disagree...
blocking-b2g: 1.3+ → 1.3?

Updated

4 years ago
Depends on: 961926
Wesley

Is this function common to both audio and video?
Flags: needinfo?(whuang)
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.
Blocks: 957937
blocking-b2g: 1.3? → ---
Flags: needinfo?(whuang)
Summary: RTSP: the seek function usually fails. → RTSP: seek function issues
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)?
Flags: needinfo?(ettseng)
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.
Flags: needinfo?(ettseng)
(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.

Updated

4 years ago
No longer blocks: 957937

Comment 19

4 years ago
It blocks 1.4 feature.
blocking-b2g: --- → 1.4+
Blocks: 957937
No longer blocks: 892360
Target Milestone: --- → 1.4 S3 (14mar)
No longer depends on: 947114

Comment 20

4 years ago
(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
This doesn't fall under the QC blocking feature list & DSDS feature list. Renoming.
blocking-b2g: 1.4+ → 1.4?

Comment 22

4 years ago
No more 1.4+ feature now.
blocking-b2g: 1.4? → ---

Updated

4 years ago
Component: General → RTSP

Comment 23

4 years ago
It is not a 1.4 feature.
Target Milestone: 1.4 S3 (14mar) → ---
Whiteboard: [ft:ril]

Updated

4 years ago
blocking-b2g: --- → backlog

Updated

4 years ago
No longer blocks: 957937

Updated

4 years ago
Blocks: 957937
No longer blocks: 929372
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.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [p=1]
Target Milestone: --- → 2.0 S2 (23may)

Comment 25

4 years ago
Good Job, Ethan! \^O^/~
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.