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

Status: VERIFIED FIXED

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

Description

[William Hsu [:whsu]]

Attachment: 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

Comment 1

[Ethan Tseng [:ethan]]

I also reproduced this bug.
The suspicious patch that caused this regression is:
https://hg.mozilla.org/mozilla-central/rev/116406daf772

Comment 2

[Ethan Tseng [:ethan]]

Attachment: bug-1015169-fix.patch

Fix a logical error that resulted in RTSP objects being not released.
A small exclamation mark matters!  :)

Comment 3

[Ethan Tseng [:ethan]]

Attachment: bug-1015169-fix.patch

The previous patch contains some carelessly included files.

Comment 4

[Ethan Tseng [:ethan]]

The result of Try server:
https://tbpl.mozilla.org/?tree=Try&rev=a8877b497adc

Comment 5

[Ethan Tseng [:ethan]]

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?

Comment 6

[Vincent Chang[:vchang][changyihsin]]

(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.

Comment 7

[Ethan Tseng [:ethan]]

Attachment: bug-1015169-fix.patch

Replace null-pointer check by assertion.

Comment 8

[Ethan Tseng [:ethan]]

The result of Try server:
https://tbpl.mozilla.org/?tree=Try&rev=c08a07cc6995

Comment 9

[Ethan Tseng [:ethan]]

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.

Comment 10

[William Hsu [:whsu]]

WoW~ Cool! Thanks Ethan!
I will reconfirm this patch after it lands on master, and check the fullrun result that US QAnalysts finished.

Comment 11

[Ethan Tseng [:ethan]]

(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.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/6b96344101a9

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6b96344101a9

Comment 14

[William Hsu [:whsu]]

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