928332 - [RTSP][Gaia] Support RTSP in Video apps.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Video, defect)

Description

[Ken Chang[:kenkai]]

Video apps need to handle the RTSP streaming.

Comment 1

[Ken Chang[:kenkai]]

Francis, who can help this bug?

Comment 2

[Francis Lee [:frlee]]

hi David,

would you be able to help on this bug? or maybe redirect to your member?

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

Francis,

You forgot to set the need info flag. Actually, I may help to finish RTSP in video app. But I still love to have the feedback from David. I also put Hema in cc list to let her know this thing.

Comment 4

[David Flanagan [:djf]]

I don't know what RTSP is, so I can't provide any info.

I'm curious where this requirement is coming from. What's the user story about this?

I don't think we have planned for this for 1.3, so depending on how much work it is we may not be able to do this for 1.3

Comment 5

[Dominic Kuo [:dkuo]]

I am also watching Gaia::Video and sorry for jumping in. Currently video app only handles storages/local files, it used to handle remote urls but removed months ago. Unless video app is going to support playing with urls again, like RTSP links, or I think currently video does not need to do anything about the RTSP streaming.

Comment 6

[Ken Chang[:kenkai]]

(In reply to David Flanagan [:djf] from comment #4)
> I'm curious where this requirement is coming from. What's the user story
> about this?
The user story is very simple, supporting RTSP in youtube.(bug 929372).
> I don't think we have planned for this for 1.3, so depending on how much
> work it is we may not be able to do this for 1.3
This feature is in 1.3 backlog.

Comment 7

[Ken Chang[:kenkai]]

According to comment 3, assign to John. If you think it isn't appropriate, please reassign to Nobody..:)

Comment 8

[Sotaro Ikeda [:sotaro]]

From Comment 5, we should not implement it until we have a concrete user story about it and decided is as product requirement, I think.

Comment 9

[Ken Chang[:kenkai]]

Vincent, can you please comment in order to let Gaia developer know the current status?
Sandip, Gaia developer needs concrete user story. Can you please provide?

Comment 10

[Vincent Chang[:vchang][changyihsin]]

Since bug 831645 is landed, we could support rtsp link with html5 video tag in b2g (Android ICS) right now. Below is an example. 

<video id=test controls preload=none width=300 height=300>
  <source src="rtsp://xxx.xxx.xxx.xxx/video.3gp"/>
</video>

To support rtsp in url bar(Bug 884702), our idea is to tell the video app that there is a Rtsp request from browser app, so that video app uses video tag and rtsp url to play rtsp streaming, make sense ?

Comment 11

[John Hu [:johnhu][:johu][:醬糊小弟]]

Agree with dominic.

To support this, we may need to modify the manifest to support rtsp protocol. As per comment 10, we already have similar mechanism for playing the video. The caller just calls "open" activity of video app to player the video. The activity message should contains:
data: {
  url: "rtsp://xxx.xxx.xxx.xxx/video.3gp",
  type: "mime-type, video/3gpp", (optional)
  title: "the title of video", (optional)
  filename: "if you want to save the file, give us a file name. this must have correct extension with mime type." (optional)
}


(In reply to Dominic Kuo [:dkuo] from comment #5)
> I am also watching Gaia::Video and sorry for jumping in. Currently video app
> only handles storages/local files, it used to handle remote urls but removed
> months ago. Unless video app is going to support playing with urls again,
> like RTSP links, or I think currently video does not need to do anything
> about the RTSP streaming.

Comment 12

[Ken Chang[:kenkai]]

(In reply to John Hu [:johnhu] from comment #11)
> Agree with dominic.
> 
> To support this, we may need to modify the manifest to support rtsp
> protocol. As per comment 10, we already have similar mechanism for playing
> the video. The caller just calls "open" activity of video app to player the
> video. The activity message should contains:
> data: {
>   url: "rtsp://xxx.xxx.xxx.xxx/video.3gp",
>   type: "mime-type, video/3gpp", (optional)
>   title: "the title of video", (optional)
>   filename: "if you want to save the file, give us a file name. this must
> have correct extension with mime type." (optional)
> }
Ethan, can you please have a try?

Comment 13

[Ethan Tseng [:ethan]]

Hi John Hu and Ken,

Since we are sending RTSP request from gecko, we should not use activity message. Instead, we broadcast system messages with a special type "rtsp-open-video", which will be implemented in bug 884702. We will carry the RTSP link within the "url" property of the data object.
I already tried to modify the manifest of video app and successfully launched the video app with our broadcasted message. However, the video app could not play RTSP streaming video. It seems the media element did not fire the "onloadedmetadata" or "onloadeddata" event to the video app.

Comment 14

[John Hu [:johnhu][:johu][:醬糊小弟]]

reset to nobody because I am currently focused other higher priority bugs.

Comment 15

[Francis Lee [:frlee]]

hi Hema,

could you please help to assign someone to help on this bug?

Comment 16

[Hema Koka [:hema]]

Francis,

Media Apps team has NO bandwidth to take on additional work for 1.3

Thanks
Hema

Comment 17

[Sandip Kamat]

(In reply to Ken Chang from comment #9)
> Vincent, can you please comment in order to let Gaia developer know the
> current status?
> Sandip, Gaia developer needs concrete user story. Can you please provide?

Currently Waiting on partners to clarify what they need via partner management.

Comment 18

[Ethan Tseng [:ethan]]

(In reply to Ethan Tseng [:ethan] from comment #13)
> ... However, the video app
> could not play RTSP streaming video. It seems the media element did not fire
> the "onloadedmetadata" or "onloadeddata" event to the video app.

Update the current status.
John Hu and I are working in progress and we further verified that "onloadedmetadata" and "onloadeddata" events are actually fired from media element to video app. We didn't notice them.
Although not so smoothly, we can successfully play RTSP video in an <a href> link right now.

Comment 19

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Thanks for the update Ethan.
Looks like we are having positive progress so I'm setting target milestone to sprint6.
Let me know if it's inappropriate.

Comment 20

[John Hu [:johnhu][:johu][:醬糊小弟]]

Actually, it fires when we press "play" button. That's not as expected. It should load metadata when we set the url like local file and other streaming format.

Comment 21

[Dominic Kuo [:dkuo]]

(In reply to John Hu [:johnhu] from comment #20)
> Actually, it fires when we press "play" button. That's not as expected. It
> should load metadata when we set the url like local file and other streaming
> format.

Maybe you can try to set "preload=metadata" to the video tag, not sure if it helps.

Comment 22

[John Hu [:johnhu][:johu][:醬糊小弟]]

It already had this attribute. That's why I feel the behavior is not as expected as others. Thanks for this, anyway.

Comment 23

[Ethan Tseng [:ethan]]

Hi George, bug 884702 was landed.
The message type and format are as followed.

Message type: "rtsp-open-video"
Message content
data: {
  url: "rtsp://some-url-to-rtsp",
  title: "rtsp://some-url-to-rtsp",
  type: "video/rtsp"
}

There are only 3 properties in this message.
The MIME type is a fake one. I don't know whether it concerns you or not.

Comment 24

[Punam Dahiya [:pdahiya]]

Synched up with John Hu, George in Taipei gaia team is helping with this bug.

Comment 25

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

Comment 26

[Jason Smith [:jsmith]]

RTSP is not a committed feature for 1.3 - it's targeted. This should not block the release.

Comment 27

[Ken Chang[:kenkai]]

George, I wonder if your patch is ready for review? Since you don't add a reviewer.

Comment 28

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Ken,

I haven't tested the function yet since it requires bug 938512 to be ready.

Hi Ethen,

is there any way for me to test it now?

Comment 29

[Ethan Tseng [:ethan]]

(In reply to George Duan [:gduan] from comment #28)
> Hi Ken,
> I haven't tested the function yet since it requires bug 938512 to be ready.
> Hi Ethen,
> is there any way for me to test it now?
You can still test it without bug 938512. Pressing "play" button will fire the loadedmetadata event.

Comment 30

[John Hu [:johnhu][:johu][:醬糊小弟]]

Ethan,

We can do the test and review, but I prefer to land the code until bug 938512 landed.

Comment 31

[Ethan Tseng [:ethan]]

(In reply to John Hu [:johnhu] from comment #30)
> Ethan,
> We can do the test and review, but I prefer to land the code until bug
> 938512 landed.
Sure. Bug 938512 is in review and I think will be landed soon.

Comment 32

[[:fabrice] Fabrice Desré]

Using a system message (rtsp-open-video) is not what we should have done here. Is there any reason to not fire an activity instead? What will happen once another app declares that it supports the rtsp-open-video system message?

Until this is solved I'm vetoing landing that on 1.3.

Comment 33

[John Hu [:johnhu][:johu][:醬糊小弟]]

Fabrice,

I prefer to use activity, too. But as per comment 13, Ethan said we should not use activity to do so. And that system message (rtsp-open-video) had already landed to gecko with bug 884702. This is the corresponding gaia bug.

Comment 34

[John Hu [:johnhu][:johu][:醬糊小弟]]

Sorry, there is a typo here:

I prefer *not* to land the code until bug 938512 landed.

(In reply to John Hu [:johnhu] from comment #30)
> Ethan,
> 
> We can do the test and review, but I prefer to land the code until bug
> 938512 landed.

Comment 35

[Gene Lian (I already quit Mozilla)]

(In reply to Fabrice Desré [:fabrice] from comment #32)
> Using a system message (rtsp-open-video) is not what we should have done
> here. Is there any reason to not fire an activity instead? What will happen
> once another app declares that it supports the rtsp-open-video system
> message?

Activity would still have the same issue. Right? Arbitrary apps can claim to be able to receive an activity with a specific activity.name (rtsp-open-video).

Using the system message makes much sense to me than activity. As you know:

  - Activity:       app -> platform -> app
  - System message: platform -> app

If the platform wants to notify Gaia about something, it is obvious to fire the system message. Hacking to send an activity without the request from Gaia (i.e., new MozActivity()) is problematic: what would happen if the subscriber runs the activity's callback? There is no corresponding publisher handling the result.

Does that sound reasonable?

Comment 36

[Ethan Tseng [:ethan]]

Hi Fabrice,
How do you think about Gene's comment?

Comment 37

[[:fabrice] Fabrice Desré]

If you use a system message like you do, it is broadcasted to all apps declaring this message. This means that all these apps will be launched and will try to load the stream. That makes no sense at all. Also, gaia is not moving apps to the foreground when they get a system message. 

On the other hand, using an activity means that the user get the opportunity to decide which app will play the stream (he will choose GeneWonderfulPlayer for sure!), and only this app will get the stream url. The callback is a non-issue: nothing prevents the video app to send back a result, it will just be ignored by the platform. We did that for the youtube handler, and are still using that for mime-type based handler, and this works fine.

Comment #13 only says "we will use system messages" but gives no justification. I think this was a mistake and should have never landed.

Comment 38

[Hema Koka [:hema]]

This is not a committed feature for 1.3, hence not blocking it. 

Also NI'd Sri for product input on this bug (as I just heard during the triage that we dont need to do anything on video for RTSP)

Comment 39

[John Hu [:johnhu][:johu][:醬糊小弟]]

The open activity already supports streaming source. If gecko can use activity to call video app, we don't need to do anything to support RTSP because it already does.

Comment 40

[Ethan Tseng [:ethan]]

(In reply to Fabrice Desré [:fabrice] from comment #37)
> Comment #13 only says "we will use system messages" but gives no
> justification. I think this was a mistake and should have never landed.

Hi Fabrice,
Thanks for your clear explanation. I will fix this problem as soon as possible.

Comment 41

[Ethan Tseng [:ethan]]

Attachment: PR to master

Add type "video/rtsp" for "view" activity of video app.

Comment 42

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8344630 [details] [review]
PR to master

Thanks for this patch. I am happy to see RTSP to use activity to open video app because we already have the codes to handle streaming content.

Comment 43

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to master:
https://github.com/mozilla-b2g/gaia/commit/ff703bf2c0baaf06b2b74144bec9b9cc99f22404

Comment 44

[Jason Smith [:jsmith]]

(In reply to Hema Koka [:hema] from comment #38)
> This is not a committed feature for 1.3, hence not blocking it. 
> 
> Also NI'd Sri for product input on this bug (as I just heard during the
> triage that we dont need to do anything on video for RTSP)

See this comment above. This is not a blocker for the same reasons already cited in this comment.

Comment 45

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

(In reply to Jason Smith [:jsmith] from comment #44)
> (In reply to Hema Koka [:hema] from comment #38)
> > This is not a committed feature for 1.3, hence not blocking it. 
> > 
> > Also NI'd Sri for product input on this bug (as I just heard during the
> > triage that we dont need to do anything on video for RTSP)
> 
> See this comment above. This is not a blocker for the same reasons already
> cited in this comment.

We are being requested to land RTSP in 1.3 for BD purpose.
Also requesting approval to uplift.

Comment 46

[[:fabrice] Fabrice Desré]

I'm fine with uplifting that to 1.3 because it's a trivial patch.

In general, if BD absolutely needs something in a release, they must first get that to be a committed feature.

Comment 47

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to 1.3:
https://github.com/mozilla-b2g/gaia/commit/78971e896592a4896d0daa6a35c0dc3b4c6e8b32