This bug aims to establish the basic test case for RTSP feature.

Related bug: Bug 1003711 - [RTSP] Build Darwin Streaming Server on B2G emulator

A WIP patch that only starts up DSS.

A WIP patch.

A WIP patch.

Hi Steve, I am trying to write RTSP unit test on marionette framework, and we've got a problem. Marionette can be executed only in chrome process, not content (It's limited by several reasons, however, that's the fact for now). I guess you remember that our RtspChannel is only implemented in the child side, while the RtspChannelParent exists just for HTTP->RTSP redirection and nothing else. This puts a limitation to our RTSP function that it can only be initialized from the content process. Marionette test cannot load it. I think HttpChannel doesn't have this limitation. It can be loaded in both content and chrome process. When I was refactoring RtspChannel in bug 992568, I was not aware of this OOP issue. It seems we should refactor it again to make it work in chrome process (OOP = false). I'd like to know what you think.

Hi Ethan, Are you sure marionette can't be run in content? MDN says that the context defaults to content - https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette/Marionette_JavaScript_Tests see MARIONETTE_CONTEXT. If this page is not updated, then I think you might consider writing some stub functions to send messages from chrome to content. I don't think we want to start refactoring things back to the chrome process just for tests. I'm not excited about stub functions either :) but I'd rather have them than a big refactoring.

(In reply to Steve Workman [:sworkman] from comment #6) > Hi Ethan, > > Are you sure marionette can't be run in content? MDN says that the context > defaults to content - > https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette/ > Marionette_JavaScript_Tests see MARIONETTE_CONTEXT. Hi Steve, thanks for you suggestion. After reading your comment and rethinking the problem, I found I should not use the terms "chrome vs content process", which are confusing here. Instead, the problem should be "parent vs child process". We developed and tested RTSP on B2G only, which enables oop (out-of-process) mode. But when we run Marionette test, oop mode is off (oop = false). RtspChannel will be created by a parent process instead of a child process but we don't support that. IsNeckoChild() XRE_GetProcessType() Browser App true (1) GeckoProcessType_Content (2) Marionette Test false (0) GeckoProcessType_Default (0) Do you think we should fix this to make RTSP work without oop mode? p.s. You are right. Marionette test codes can be run in content process as its context.

(In reply to Ethan Tseng [:ethan] from comment #7) > But when we run Marionette test, oop mode is off (oop = false). BTW, bug 970804 added support to marionette tests running in oop mode. I will also try to utilize this support as a workaround for tests. Not sure it's feasible or not. (I tried to set "oop = true" in my marionette test but the test script failed to run).

Removing needinfo as we talked about this offline.

(In reply to Ethan Tseng [:ethan] from comment #8) > BTW, bug 970804 added support to marionette tests running in oop mode. > I will also try to utilize this support as a workaround for tests. > Not sure it's feasible or not. > (I tried to set "oop = true" in my marionette test but the test script > failed to run). Bug 1007062 - SpecialPowers doesn't work in Marionette OOP mode Bug 1058158 - Marionette WebAPI tests should run in either a test container or a clean environment These two bugs were fixed and realized the support of SpecialPowers in marionette OOP mode. Now we can use "test_container = true" to enable OOP mode in marionette test. Basically, RTSP stream can be loaded under this situation. More follow-ups are needed to complete the test case.

Unexpected error happens while loading an RTSP stream in marionette test.

Hi Blake and Bruce, I am trying to execute a marionette test that loads RTSP stream and the test case will be passed when it receives the ended event. However, an unexpected error always happens in the middle of stream. Do you have any idea or clue on how to debug this issue? The media is H.264 baseline profile. I/OMXCodec( 287): [OMX.google.h264.decoder] AVC profile = 66 (Baseline), level = 30 I/OMXCodec( 287): [OMX.google.h264.decoder] video dimensions are 320 x 240 I/OMXCodec( 287): [OMX.google.h264.decoder] Crop rect is 320 x 240 @ (0, 0) The logcat is attached. It seems no much useful information in the logcat output. I plan to make a debug build to generate more logs.

Add addEventListener() to wait for the ended event.

BTW, I used the same test case to load an HTTP stream media (also H.264 with baseline profile). It can successfully get the ended event and the test case passed. I guess both RTSP and HTTP stream in marionette test are processed by the same software decoder. The unexpected error (crash) might indicate an exception in our RTSP stack. I'll figure it out. Do you have experience to play H.264 media in marionette test?

Bug 1086198 is found using the WIP patch in this bug.

The JS test script cannot receive "ended" media event. Sometimes it even crashes when the playback reaches the end of media. Bug 1088538 was filed to track the crash issue.

Per discussion offline, it looks currently video can be played. Good to know!

(In reply to Ethan Tseng [:ethan] from comment #12) > I am trying to execute a marionette test that loads RTSP stream and the test > case will be passed when > it receives the ended event. > However, an unexpected error always happens in the middle of stream. We found that the occasional crash or timeout are caused by our RTSP code. We are working on these issues in other bugs. The only problem with marionette is that, the video frames cannot be rendered on the screen. For automation test purpose, I'll focus on verifying media events and ignore rendering issue for now.

The draft RTSP test script. It contains only one basic test case for video H.264. Henry, Could you provide some feedback for this patch? Should we wrapDomRequestAsPromise() for any DOM manipulations? I didn't wrap document.createElement() in this patch.

Comment on attachment 8514166 [details] [diff] [review] bug-1032111-wip.patch As Hsin-Yi mentioned offline, bug 839838 implemented DOMRequest.then(). We don't need to wrap DOMRequest as promise by ourselves. Reference: http://hg.mozilla.org/mozilla-central/rev/a14b8bf72b16 So I cancel the feedback request. I'll update my patch according to this information.

Refine the patch. Remove codes to wrap DOMRequest. Hi Henry, Could you provide some feedback? Thanks!

*** Memorandum *** (In reply to Ethan Tseng [:ethan] from comment #10) > Bug 1007062 - SpecialPowers doesn't work in Marionette OOP mode > Bug 1058158 - Marionette WebAPI tests should run in either a test container > or a clean environment > These two bugs were fixed and realized the support of SpecialPowers in > marionette OOP mode. > Now we can use "test_container = true" to enable OOP mode in marionette test. Bug 1088538 - [RTSP] System crash in the end of playback due to NULL accessUnit This bug fixed the crash issue after end-of-stream. So RTSP test on marionette can receive the "ended" media event now.

Comment on attachment 8514854 [details] [diff] [review] bug-1032111-wip.patch Review of attachment 8514854 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Ethan! Looks good in general. Some comments are left for discussion. ::: netwerk/protocol/rtsp/test/marionette/test_rtsp_video_h264.js @@ +1,5 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this file, > + * You can obtain one at http://mozilla.org/MPL/2.0/. */ > + > +MARIONETTE_TIMEOUT = 300000; // 300 sec I am not sure if this timeout is too long .... @@ +9,5 @@ > + > +gTestSuite.doTest(function () { > + return Promise.resolve() > + .then(() => gTestSuite.ensureWifiEnabled(false)) > + .then(() => gTestSuite.requestWifiEnabled(true)) Why not just .then(() => gTestSuite.ensureWifiEnabled(true)) ? @@ +10,5 @@ > +gTestSuite.doTest(function () { > + return Promise.resolve() > + .then(() => gTestSuite.ensureWifiEnabled(false)) > + .then(() => gTestSuite.requestWifiEnabled(true)) > + .then(() => gTestSuite.startDarwinStreamingServer()) Whenever there're other test cases, we can move startDarwinStreamingServer/stopDarwinStreamingServer into doTest function. But it's okay at this time since there's only one test case. @@ +13,5 @@ > + .then(() => gTestSuite.requestWifiEnabled(true)) > + .then(() => gTestSuite.startDarwinStreamingServer()) > + .then(() => gTestSuite.loadVideoElement(URL)) > + .then(() => gTestSuite.waitForMediaEventOnce('loadeddata')) > + .then(() => gTestSuite.waitForMediaEventOnce('ended')) The event is likely to be fired before we begin to wait. We should probably push all these promises to an array and use "Promise.all()" to prevent from the racing.

(In reply to Henry Chang [:henry] from comment #23) > ::: netwerk/protocol/rtsp/test/marionette/test_rtsp_video_h264.js > @@ +1,5 @@ > > +MARIONETTE_TIMEOUT = 300000; // 300 sec > I am not sure if this timeout is too long .... The video clip being tested is 70-sec long. In real tests it takes about 72~73 seconds to complete the media playback. Setting the timeout as 120 sec should be quite safe enough. > @@ +9,5 @@ > > +gTestSuite.doTest(function () { > > + return Promise.resolve() > > + .then(() => gTestSuite.ensureWifiEnabled(false)) > > + .then(() => gTestSuite.requestWifiEnabled(true)) > Why not just > .then(() => gTestSuite.ensureWifiEnabled(true)) You are right! I'll change this. > @@ +10,5 @@ > > +gTestSuite.doTest(function () { > > + return Promise.resolve() > > + .then(() => gTestSuite.ensureWifiEnabled(false)) > > + .then(() => gTestSuite.requestWifiEnabled(true)) > > + .then(() => gTestSuite.startDarwinStreamingServer()) > Whenever there're other test cases, we can move > startDarwinStreamingServer/stopDarwinStreamingServer > into doTest function. But it's okay at this time since there's only one test > case. Hmm... Actually both Wifi and streaming server are prerequisites for RTSP tests. I'll move both of them into the doTest function. Then all test cases don't have to bother with them. > @@ +13,5 @@ > > + .then(() => gTestSuite.requestWifiEnabled(true)) > > + .then(() => gTestSuite.startDarwinStreamingServer()) > > + .then(() => gTestSuite.loadVideoElement(URL)) > > + .then(() => gTestSuite.waitForMediaEventOnce('loadeddata')) > > + .then(() => gTestSuite.waitForMediaEventOnce('ended')) > The event is likely to be fired before we begin to wait. We should probably > push all these promises to an array and use "Promise.all()" to prevent from > the racing. I didn't think of this! Thanks for your advice. I'll try it!

Refinement according to Henry's suggestions.

Comment on attachment 8517289 [details] [diff] [review] bug-1032111.patch Henry and Steve, Could you help to review the patch?

waitForMediaEventOnce(), which calls addEventListener(), should be called before the start of the playback. So we can prevent racing problem addressed in comment 23.

Comment on attachment 8517361 [details] [diff] [review] bug-1032111.patch Review of attachment 8517361 [details] [diff] [review]: ----------------------------------------------------------------- Looks good as a starting point for more test cases. r=me. ::: netwerk/protocol/rtsp/test/marionette/test_rtsp_video_h264.js @@ +12,5 @@ > + return Promise.all([ > + gTestSuite.createVideoElement(URL), > + gTestSuite.waitForMediaEventOnce('loadeddata'), > + gTestSuite.waitForMediaEventOnce('ended'), > + gTestSuite.playVideoElement() I'm not 100% familiar with Promises, but it looks like only waitForMediaEventOnce returns a promise in the list here. createVideoElement returns true, and playVideoElement doesn't have a return type. Is this ok? Also, did you want to add pause here too?

(In reply to Steve Workman [:sworkman] from comment #28) > I'm not 100% familiar with Promises, but it looks like only > waitForMediaEventOnce returns a promise in the list here. createVideoElement > returns true, and playVideoElement doesn't have a return type. Is this ok? I make createVideoElement() always return true because I assume its operations always succeed. Henry, what do you think? Do we have to return "Promise.resolve()" instead?

(In reply to Ethan Tseng [:ethan] from comment #29) > (In reply to Steve Workman [:sworkman] from comment #28) > > I'm not 100% familiar with Promises, but it looks like only > > waitForMediaEventOnce returns a promise in the list here. createVideoElement > > returns true, and playVideoElement doesn't have a return type. Is this ok? > > I make createVideoElement() always return true because I assume its > operations always succeed. > Henry, what do you think? > Do we have to return "Promise.resolve()" instead? Returning |true| is good enough. However, you have to put gTestSuite.playVideoElement() outside of Promise.all() such as: return Promise.all([ gTestSuite.waitForMediaEventOnce('loadeddata'), gTestSuite.waitForMediaEventOnce('ended'),] gTestSuite.createVideoElement(URL), .then(gTestSuite.playVideoElement()); Promise.all() itself would also return a promise which get resolved whenever all the associated promises have been resolved. Besides, you will have to call waitForMediaEventOnce() before the event gets fired. Otherwise, you might miss the event.

Refine the patch according to review's comments.

Henry, I just made two changes in the latest patch. 1. Return values of createVideoElement() and playVideoElement() Instead of returning |true|, make them return |Promise.resolve()|, which is more clear and is "thenable". 2. Elements in Promise.all() createVideoElement(), waitForMediaEventOnce('loadeddata') and playVideoElement() are wrapped in Promise.all(). When they are all resolved, then we do waitForMediaEventOnce('ended'). I think this is more clear and should eliminate the racing issue. What do you think?

As discussed offline, moving the waitForEvent function into the array of Promise.all() is still a safer way.

Add RTSP test case into marionette-webapi set.

I wanna try the test case, but it seems Treeherder doesn't perform any marionette-webapi test. The syntax: try: -b do -p all -u marionette-webapi -t none The result: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=036464f24afb Ryan, can you help to take a look and see what's wrong with it?

I noticed the "Mnw" would flash for a short period of time in the row "B2G ICS Emulator opt". But it soon disappears.

Oh, we can see the Mnw busted information from TBPL: https://tbpl.mozilla.org/?tree=Try&rev=036464f24afb

The Try result reports ScriptTimeoutException in Wifi test case (test_wifi_tethering_enabled.js). So I temporarily removed test cases of Wifi and tethering. Then the same timeout exception raises for another script (test_cellbroadcast_umts.js). https://tbpl.mozilla.org/?tree=Try&rev=49a2dbdb4239 I think something is wrong in my test script. Need to find it out. Henry and Chuck, could you provide some suggestion?

According the following log, it might be due to the failure of running a test case in test container... test_cellbroadcast_umts.js was passed in fact. 03:03:13 INFO - Traceback (most recent call last): 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runtests.py", line 40, in <module> 03:03:13 INFO - cli() 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runtests.py", line 35, in cli 03:03:13 INFO - runner = startTestRunner(runner_class, options, tests) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runtests.py", line 20, in startTestRunner 03:03:13 INFO - runner.run_tests(tests) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runner/base.py", line 742, in run_tests 03:03:13 INFO - self.run_test_sets() 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runner/base.py", line 926, in run_test_sets 03:03:13 INFO - self.run_test_set(self.tests) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runner/base.py", line 907, in run_test_set 03:03:13 INFO - self.run_test(test['filepath'], test['expected'], test['test_container']) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runner/base.py", line 881, in run_test 03:03:13 INFO - self.launch_test_container() 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/runner/base.py", line 687, in launch_test_container 03:03:13 INFO - }""", script_timeout=60000) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/marionette.py", line 1295, in execute_async_script 03:03:13 INFO - filename=os.path.basename(frame[0])) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/decorators.py", line 36, in _ 03:03:13 INFO - return func(*args, **kwargs) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/marionette.py", line 643, in _send_message 03:03:13 INFO - self._handle_error(response) 03:03:13 INFO - File "/builds/slave/test/build/tests/marionette/marionette/marionette.py", line 705, in _handle_error 03:03:13 INFO - raise errors.ScriptTimeoutException(message=message, status=status, stacktrace=stacktrace) 03:03:13 INFO - errors.ScriptTimeoutException: ScriptTimeoutException: timed out 03:03:13 ERROR - Return code: 1

Comment on attachment 8519845 [details] [diff] [review] RTSP automation test Review of attachment 8519845 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!

Discussed offline with Edgar and Henry. RTSP test script must run in OOP mode (test_container = true). However, once marionette test has performed in-process mode test, it can no longer perform tests in OOP mode. Bug 1075437 is aimed to fix this problem by re-ordering test scripts to run OOP mode tests first. Edgar, correct me if I misunderstand anything.

I try to run this test on my PC, and it always timeout. I put some log in HTMLMediaElement::Play() and it turns out that its mDecoder is NULL.

It turns out that my command was wrong. I used ./mach marionette-webapi mozilla-central/netwerk/protocol/rtsp/test/marionette/test_rtsp_video_h264.js That doesn't enable OOP mode, the correct command is ./mach marionette-webapi mozilla-central/netwerk/protocol/rtsp/test/marionette/manifest.ini

No one is actually working on RTSP for FxOS right now.