Closed Bug 1015169 Opened 7 years ago Closed 7 years ago

[RTSP][V2.0] Cannot play RTSP streaming after reloading web page

Categories

(Firefox OS Graveyard :: RTSP, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed)

VERIFIED FIXED
2.0 S3 (6june)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: whsu, Assigned: ethan)

Details

(Keywords: regression, Whiteboard: [p=1])

Attachments

(2 files, 2 obsolete files)

Attached video WP_20140523_008.mp4
* Description:
  This might be a regression bug. The older V2.0  build doesn't have this problem.
  After you stop the RTSP streaming via browser "X" icon then reload the webpage, the built-in media player hangs there and wrong timestamp shows on toolbar.
  Attach the demo video.(WP_20140523_008.mp4)

* Reproduction steps:
  2. Launch the following page via browser
     - http://goo.gl/BhJd77
  3. Tap the "Video test page" link
  4. Tap "X" icon of URL bar to stop RTSP streaming
  5. Tap reload icon of URL bar to reload webpage

* Expected result:
  Built-in media player can play RTSP streaming

* Actual result:
  1. The built-in media player cannot play RTSP streaming
  2. The wrong timestamp shows on toolbar

* Reproduction build: V2.0 (Buri)
 - Gaia      7db23414f2d632f4d00b5023ac1090b6045dc5fd
 - Gecko     https://hg.mozilla.org/mozilla-central/rev/2619a4def1b9
 - BuildID   20140522160203
 - Version   32.0a1
Assignee: nobody → ettseng
blocking-b2g: --- → 2.0?
I also reproduced this bug.
The suspicious patch that caused this regression is:
https://hg.mozilla.org/mozilla-central/rev/116406daf772
blocking-b2g: 2.0? → 2.0+
Attached patch bug-1015169-fix.patch (obsolete) — Splinter Review
Fix a logical error that resulted in RTSP objects being not released.
A small exclamation mark matters!  :)
Attachment #8429120 - Flags: review?(vchang)
Attached patch bug-1015169-fix.patch (obsolete) — Splinter Review
The previous patch contains some carelessly included files.
Attachment #8429120 - Attachment is obsolete: true
Attachment #8429120 - Flags: review?(vchang)
Attachment #8429125 - Flags: review?(vchang)
Attachment #8429125 - Flags: review?(vchang) → review+
Hi Vincent,
After some consideration around this line, I think we should consider what Benjamin mentioned today. That is, we should first find out when and why mHandler would become NULL here, then decide to apply whether a simple null-pointer check or a sane fix.
So, I recommend we replace this null-pointer check by an assertion, which could help us identify the scenario more easily.
How do you think?
Severity: normal → critical
Status: NEW → ASSIGNED
Keywords: regression
Priority: -- → P1
Whiteboard: [p=1]
Target Milestone: --- → 2.0 S3 (6june)
(In reply to Ethan Tseng [:ethan] from comment #5)
> Hi Vincent,
> After some consideration around this line, I think we should consider what
> Benjamin mentioned today. That is, we should first find out when and why
> mHandler would become NULL here, then decide to apply whether a simple
> null-pointer check or a sane fix.
> So, I recommend we replace this null-pointer check by an assertion, which
> could help us identify the scenario more easily.
> How do you think?


That's a good idea, go for it.
Replace null-pointer check by assertion.
Attachment #8429125 - Attachment is obsolete: true
Attachment #8430495 - Flags: review+
I tested this patch manually:
1. The symptom of this bug is fixed.
2. No crash happened with the added assertion (under many combination of user scenarios).

Finally, the result of Try server looks good.
Request to check in this patch.
Keywords: checkin-needed
WoW~ Cool! Thanks Ethan!
I will reconfirm this patch after it lands on master, and check the fullrun result that US QAnalysts finished.
(In reply to William Hsu [:whsu] from comment #10)
> WoW~ Cool! Thanks Ethan!
> I will reconfirm this patch after it lands on master, and check the fullrun
> result that US QAnalysts finished.

You're welcome.
And we still look to your tests to verify it. :)

BTW, after research around this bug, we found some potential issue of the "end-of-stream" scenario.
Bug 951278 (The RTSP streaming always stops at the last 1st or 2nd second) and bug 1006484 (Cannot replay RTSP streaming) added some mechanism to allow us to re-play an RTSP media after it is played to the end.
But the mechanism is not robust enough that you might see some strange behavior under this scenario.

I already filed bug 1017444 to track this issue. If you encounter the same issue, please record any detail in this bug.
https://hg.mozilla.org/mozilla-central/rev/6b96344101a9
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Great! Cannot reproduce it on latest V2.0 build.
Mark this bug as "VERIFIED"

* Build Information:
 - Gaia      8d865839d932bfbd5e157f376f74d8cb12bfdd51
 - Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/1d4046a8cb6c
 - BuildID   20140610000223
 - Version   32.0a2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.