Closed Bug 947101 Opened 7 years ago Closed 7 years ago

Rtsp: Backward seek fail.


(Firefox OS Graveyard :: General, defect)

Gonk (Firefox OS)
Not set


(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed)

blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.3 --- fixed


(Reporter: vchang, Assigned: vchang)



(Whiteboard: [FT:RIL])


(1 file, 2 obsolete files)

Sometime backward seek may fail. The content process can't receive onMediaDataAvailable event from parent process.
Blocks: 898866
Attached patch Patch v1.0 (obsolete) — Splinter Review
When doing the backward seek, the onTrackDataAvailable() may be skipped due to incorrect mLatestPausedUnit. The reason is that kWhatPausedDone event may arrive
after performSeek() command is executed.
Assignee: nobody → vchang
Attachment #8343610 - Flags: review?(sworkman)
Comment on attachment 8343610 [details] [diff] [review]
Patch v1.0

Review of attachment 8343610 [details] [diff] [review]:

In general, looks fine - I just have one query.

Also, while you're doing this, can you add in a couple of small fixes? In RtspSource::performPause and performSeek, RtspSource::mLatestPausedUnit is reset repeatedly inside the for loop. This only needs to be done once, outside the for loop. Can you change those lines please?

::: netwerk/protocol/rtsp/rtsp/RTSPSource.cpp
@@ +315,5 @@
> +            // because of previous performPause() command.
> +            for (size_t i = 0; i < mTracks.size(); ++i) {
> +                TrackInfo *info = &mTracks.editItemAt(i);
> +                info->mLatestPausedUnit = 0;
> +            }

Do you need to reset RtspConnectionHandler::mLatestPausedUnit here as well?
Attached patch Patch v.1.1 (obsolete) — Splinter Review
Address the review comments.
Attachment #8343610 - Attachment is obsolete: true
Attachment #8343610 - Flags: review?(sworkman)
Attachment #8344434 - Flags: review?(sworkman)
This is a very important fix for Rtsp seek function. It would be better if we could land it on 1.3. 
Nominate it for madai?
blocking-b2g: --- → madai?
Comment on attachment 8344434 [details] [diff] [review]
Patch v.1.1

Review of attachment 8344434 [details] [diff] [review]:

Thanks for the changes! r=me.
Attachment #8344434 - Flags: review?(sworkman) → review+
Attached patch Patch v1.2Splinter Review
Rebase the patch.
Attachment #8344434 - Attachment is obsolete: true
Rtsp should be landed to 1.3, and seek is the basic function of Rtsp streaming.
blocking-b2g: madai? → 1.3?
Closed: 7 years ago
Resolution: --- → FIXED
RTSP defect to be fixed in 1.3
blocking-b2g: 1.3? → 1.3+
Whiteboard: [FT:RIL]
You need to log in before you can comment on or make changes to this bug.