1038046 - [dolphin][flame][RTSP] RTSP cannot play some H263 video streaming

Status: VERIFIED FIXED

(Firefox OS Graveyard :: RTSP, defect)

Description

[Peipei Cheng (needinfo if you need my action)]

Attachment: browser.log

Steps to reproduce
---------------------------------------------------------------------------
1. Launch Browser
2. open http://116.228.149.59/streaming/video/3gp/video_3gp_index.html
3. Tap on any video file
   --> You will see the video play panel appears. But it keeps loading forever. When screen time out, it will be closed and return to the previous webpage.

This can be reproduced on both Flame v1.4 and dolphin.

attached is abdlog. test starts: 07-14 14:54:28.713

Test build
------------------------------------------------------------------------
Gaia      b0e9b4bdb39c5eb93a6783a34624ffc84f62b126
Gecko     55a0cfe90ffaa9d5c8a0c163e8bc14e8487af413
BuildID   20140710102927
Version   30.0
ro.build.version.incremental=282
ro.build.date=Thu Jul 10 10:27:07 CST 2014

Comment 1

[Peipei Cheng (needinfo if you need my action)]

Flame v1.3 base image also has this problem.

Comment 2

[Keven Kuo [:kkuo]]

Hi! Vincent, Ethan,

Per our discussion, please take a look. Thanks

--
Keven

Comment 3

[Vincent Chang[:vchang][changyihsin]]

The link doesn't work for my desktop vlc and galaxy s2.

Comment 4

[Ethan Tseng [:ethan]]

Attachment: bug-1038046_rtsp_rtp.pcap

My VLC player desktop and Android phone cannot play these links either.
The attached is the captured packets (only RTSP/RTCP/RTP filtered) on desktop while VLC player was trying to play one link.
The packets reveal that RTSP session was established, but the RTSP client never receive RTP stream data. Could be there a firewall in front of this server?

Comment 5

[Peipei Cheng (needinfo if you need my action)]

james, per comment 4, could you please ask sprd QA to check their test server to see if there is a firewall on that server?

Comment 6

[James Zhang (Spreadtrum)]

Ming, please help to check it.

Comment 7

[Ivan Tsay (:ITsay)]

Remove nomination for now until there is clarification on the test environment from partner.

Comment 8

[Ming]

I checked

it couldn't read resources on http://116.228.149.59.

The right server address should be http://114.30.40.60:8800.

But it failed to play videos on page of http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html,and just showed the error msg.

Comment 9

[Ming]

Attachment: log1

Comment 10

[Ming]

Hi Ethan Tseng
https://bugzilla.mozilla.org/show_bug.cgi?id=990908
this bug fixed a rtsp problem, do we need this patch?

Comment 11

[Ming]

I test  with the patch https://bugzilla.mozilla.org/attachment.cgi?id=8412340, but things have no change

Comment 12

[Ethan Tseng [:ethan]]

(In reply to Ming from comment #10)
> Hi Ethan Tseng
> https://bugzilla.mozilla.org/show_bug.cgi?id=990908
> this bug fixed a rtsp problem, do we need this patch?

Ming,
I think bug 990908 is fixing another problem irrelevant to this bug.
I will try to debug using the updated link you specify in comment 8 to see what's going on.

Comment 13

[Ethan Tseng [:ethan]]

*** Notes ***
I tried to play one RTSP link from the page http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html, such as:
rtsp://114.30.40.60:80/video/3gp/h263_aac_3gp/h263_35k_aac_18k_10f_qcif.3gp
My Flame (B2G 2.1) crashed after around 10 seconds.

By observing the packets by playing the same link with VLC desktop, RTSP/RTP packets are wrapped in HTTP. I think it is because the destination port is 80. And it would take a while (around 10 seconds) for VLC and Android phone to start to render the media. Apparently these players were trying to transport streaming over UDP in this period of time, and finally they failed over to transport over TCP.

Our RTSP already supports TCP-interleaved RTP transport (by bug 996765). However we haven't encounter the case that RTSP/RTP are wrapped in HTTP.
Need more time to research the problem and solution.

Comment 14

[Rachelle Yang [:ryang][[e-mail]]]

Hi Ethan , any update on this ? Thanks !

Comment 15

[Ethan Tseng [:ethan]]

(In reply to Rachelle Yang [:ryang][[e-mail]] from comment #14)
> Hi Ethan , any update on this ? Thanks !

I will generate a patch to address this issue. Hopefully I can make it today.

Comment 16

[Ethan Tseng [:ethan]]

(In reply to Ethan Tseng [:ethan] from comment #15)
> I will generate a patch to address this issue. Hopefully I can make it today.
Sorry, I replied this comment to the wrong place. It was for bug 1038037.

For this bug, I need more time to investigate it.

Comment 17

[Ethan Tseng [:ethan]]

Attachment: bug-1038046-crash-bt.log

GDB backtrace log.

Comment 18

[Ethan Tseng [:ethan]]

STR for the crash in comment 17.

Device    : Flame
OS Version: 2.1
Platform  : 34.0a1
Build ID  : 20140727173528
Steps:
1. Navigate to this page.
   http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html
2. Click any link in the section "VIDEO 3GP/H263 AAC 3GP".
3. Wait for about 10 seconds.
4. B2G crashes.

Comment 19

[Ethan Tseng [:ethan]]

(In reply to Ethan Tseng [:ethan] from comment #13)
> Our RTSP already supports TCP-interleaved RTP transport (by bug 996765).
> However we haven't encounter the case that RTSP/RTP are wrapped in HTTP.
> Need more time to research the problem and solution.

After some debugging work, I think RTSP/RTP wrapping inside HTTP is not a problem.
From the GDB backtrace, it seems to be another payload format issue.

Comment 20

[Wayne Chang [:wchang]]

(In reply to Ethan Tseng [:ethan] from comment #19)
> (In reply to Ethan Tseng [:ethan] from comment #13)
> > Our RTSP already supports TCP-interleaved RTP transport (by bug 996765).
> > However we haven't encounter the case that RTSP/RTP are wrapped in HTTP.
> > Need more time to research the problem and solution.
> 
> After some debugging work, I think RTSP/RTP wrapping inside HTTP is not a
> problem.
> From the GDB backtrace, it seems to be another payload format issue.

Thanks Ethan,
Can you confirm if the crash is always seen? if so we need a way to avoid crashes here.

Comment 21

[Ethan Tseng [:ethan]]

(In reply to Wayne Chang [:wchang] from comment #20)
> Thanks Ethan,
> Can you confirm if the crash is always seen? if so we need a way to avoid
> crashes here.

Yes. The crash rate is 100%.
I am already working on this.

Comment 22

[Jason.Liu]

[Blocking Requested - why for this release]:

Comment 23

[Ethan Tseng [:ethan]]

(In reply to Ethan Tseng [:ethan] from comment #13)
> By observing the packets by playing the same link with VLC desktop, RTSP/RTP
> packets are wrapped in HTTP. I think it is because the destination port is
> 80.

I have to correct my previous comments.
Actually RTSP/RTP are NOT wrapped inside HTTP.
It's just because this streaming server uses port 80 and Wireshark treats those packets as HTTP by default.

Comment 24

[Ethan Tseng [:ethan]]

Attachment: bug-1038046-wip.patch

A WIP patch that turns assertions into non-destructive normal checks.

Comment 25

[Ethan Tseng [:ethan]]

(In reply to Ming from comment #8)
> The right server address should be http://114.30.40.60:8800.
> But it failed to play videos on page of
> http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html,and just
> showed the error msg.

Hi Ming,
Please try my patch to see if it works.
It should make your Flame play the video clips in the "VIDEO 3GP/H263 AAC 3GP" section.

Comment 26

[Ethan Tseng [:ethan]]

Comment on attachment 8464022 [details] [diff] [review]
bug-1038046-wip.patch

Hi Benjamin,

As we discussed offline, while trying my WIP patch, the video quality is poor since some packets are discarded.
I found it is because the "PLEN" field in H.263+ payload header is not zero.
So I just commented out the if block for PLEN check as a workaround. But I'm not sure if it could be a long-term solution.

Please refer to RFC 4629: http://tools.ietf.org/pdf/rfc4629.pdf
Section 5.1 "General H.263+ Payload Header"

If you have any idea, please let me know.

Comment 27

[Ming]

(In reply to Ethan Tseng [:ethan] from comment #25)

> Hi Ming,
> Please try my patch to see if it works.
> It should make your Flame play the video clips in the "VIDEO 3GP/H263 AAC
> 3GP" section.

I tested with dolphin , failed with no change :(

Comment 28

[Ming]

(In reply to Ethan Tseng [:ethan] from comment #25)

> Hi Ming,
> Please try my patch to see if it works.
> It should make your Flame play the video clips in the "VIDEO 3GP/H263 AAC
> 3GP" section.

Yes , i can play videos from page of "http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/"

Comment 29

[Ming]

 (In reply to Ming from comment #28)
> 
> Yes , i can play videos from page of
> "http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/"

I can play these videos without the patch too.

I test these videos with chrome on ubuntu:
1.The rtsp videos like this can be played 
rtsp://114.30.40.60:80/video/3gp/h263_aac_3gp/h263_128k_aac_16k_10f_QCIF.3gp
2.The http videos like this can only be downloaded
http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/h263_128k_aac_16k_10f_QCIF.3gp

But on dolphin ,the http resources can be played and the rtsp resources  can not be played.

Hope it is helpful.

Comment 30

[Ethan Tseng [:ethan]]

(In reply to Ming from comment #29)
>  (In reply to Ming from comment #28)
> > Yes , i can play videos from page of
> > "http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/"
> I can play these videos without the patch too.
Ming, I am confused about your description.
The links on this page (http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/) are all HTTP streaming.
Of course they can be played without my patch, because my patch fixes an issue in RTSP streaming, not HTTP.

When I was fixing the bug, I was using the links on this page:
http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html
Which was given by you from comment 8.


> I test these videos with chrome on ubuntu:
> 1.The rtsp videos like this can be played 
> rtsp://114.30.40.60:80/video/3gp/h263_aac_3gp/h263_128k_aac_16k_10f_QCIF.3gp
Strange.
As I know, all browsers on desktop do not support RTSP scheme internally.
They would launch external applications (if registered) for rtsp://.

> 2.The http videos like this can only be downloaded
> http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/
> h263_128k_aac_16k_10f_QCIF.3gp
> But on dolphin ,the http resources can be played and the rtsp resources  can
> not be played.
So, could you specify the RTSP links you used?
Is it on this page?
http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html

Comment 31

[Ming]

(In reply to Ethan Tseng [:ethan] from comment #30)

> They would launch external applications (if registered) for rtsp://.
Yes ,it is

> So, could you specify the RTSP links you used?
> Is it on this page?
> http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html

I open the links on pc and found that links on page "video_3gp_index.html" are start with "rtsp://..." and links on page "http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/" are start with "http://..."

Comment 32

[Benjamin Chen [:bechen]]

Comment on attachment 8464022 [details] [diff] [review]
bug-1038046-wip.patch

Review of attachment 8464022 [details] [diff] [review]:
-----------------------------------------------------------------

> As we discussed offline, while trying my WIP patch, the video quality is
> poor since some packets are discarded.
> I found it is because the "PLEN" field in H.263+ payload header is not zero.
> So I just commented out the if block for PLEN check as a workaround. But I'm
> not sure if it could be a long-term solution.
> 
> Please refer to RFC 4629: http://tools.ietf.org/pdf/rfc4629.pdf
> Section 5.1 "General H.263+ Payload Header"
> 

According to the rfc4629, the PEBIT should be zero if PLEN is zero. 
So maybe you should also comment out the PEBIT too?

Comment 33

[Ethan Tseng [:ethan]]

(In reply to Benjamin Chen [:bechen] from comment #32) 
> According to the rfc4629, the PEBIT should be zero if PLEN is zero. 
> So maybe you should also comment out the PEBIT too?
Right! Thanks for your reminder.

Then I'll add a check like this:
if (PLEN == 0u && PEBIT != 0u) {
    // Discard malformed packet...
}

Comment 34

[Ethan Tseng [:ethan]]

(In reply to Ming from comment #31)
> > So, could you specify the RTSP links you used?
> > Is it on this page?
> > http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html
> I open the links on pc and found that links on page "video_3gp_index.html"
> are start with "rtsp://..." and links on page
> "http://114.30.40.60:8800/streaming/video/3gp/h263_aac_3gp/" are start with
> "http://..."

Okay. Let's clarify the issue to deal with here.
In this bug, we focus on the problem of RTSP streaming.
So, let's skip discussion about HTTP streaming to avoid confusion.

As the STR listed in comment 18, I play RTSP streaming from the top section on this page:
http://114.30.40.60:8800/Streaming/video/3gp/video_3gp_index.html

An RTSP link example:
rtsp://114.30.40.60:80/video/3gp/h263_aac_3gp/h263_128k_aac_16k_10f_QCIF.3gp

The video is in H263 payload type. Our H263 assembler failed to parse this payload type and caused B2G crash.
My patch aims to fix this problem.

From your description, it seems your Dolphin phone keeps loading without crash, right?
Please make sure your build contains the patch of bug 996765, which was landed on 2014/5/16.
We added TCP-interleaved RTP transport in that bug.

Comment 35

[Ethan Tseng [:ethan]]

Attachment: Fix H263 Assembler

Hi Steve,
Could you help to review this bug?
It is similar to bug 1038037. Just a different payload format.

Comment 36

[Ming]

(In reply to Ethan Tseng [:ethan] from comment #34)

> The video is in H263 payload type. Our H263 assembler failed to parse this
> payload type and caused B2G crash.
> My patch aims to fix this problem.
> 
> From your description, it seems your Dolphin phone keeps loading without
> crash, right?
> Please make sure your build contains the patch of bug 996765, which was
> landed on 2014/5/16.
> We added TCP-interleaved RTP transport in that bug.

Checked. v1.4 doesn't contains  bug 996765's patch.

Comment 37

[Ethan Tseng [:ethan]]

(In reply to Ming from comment #36)
> (In reply to Ethan Tseng [:ethan] from comment #34)> 
> > Please make sure your build contains the patch of bug 996765, which was
> > landed on 2014/5/16.
> > We added TCP-interleaved RTP transport in that bug.
> Checked. v1.4 doesn't contains  bug 996765's patch.

Yes! That's why your phone keeps loading.
Is it necessary to pass this scenario on v1.4?
If so, we have to request to uplift that patch to v1.4.

Comment 38

[Ming]

(In reply to Ethan Tseng [:ethan] from comment #37)
Hi ethan , after i merged bug96765's patch and attachment 8464732 [details] [diff] [review] ,  still i got the message :

Video cannot be played: either because the server or network failed or because the format is not supported

I tested on dolphin.

Comment 39

[Ethan Tseng [:ethan]]

(In reply to Ming from comment #38)
> Hi ethan , after i merged bug96765's patch and attachment 8464732 [details] [diff] [review]
> [diff] [review] ,  still i got the message :
It's bug 996765. But I guess you just made a typo here.
You merged this patch, right?
https://hg.mozilla.org/mozilla-central/rev/318e7b05b8cd
 
> Video cannot be played: either because the server or network failed or
> because the format is not supported 
> I tested on dolphin.
I think this issue is device-independent.
Perhaps I should try to reproduce it using v1.4.
Can you provide PVT build ID or Gecko revision number?

Comment 40

[Ming]

(In reply to Ethan Tseng [:ethan] from comment #39)

> You merged this patch, right?
> https://hg.mozilla.org/mozilla-central/rev/318e7b05b8cd
Yes :)

> I think this issue is device-independent.
> Perhaps I should try to reproduce it using v1.4.
> Can you provide PVT build ID or Gecko revision number?

Gaia      621d152f89347c79619aa909ad62cc2ac9d3ab5b
Gecko     62e18d231fb45f14c04bf5dd39e53a5caff6fa2c
BuildID   20140731095430
Version   30.0
ro.build.version.incremental=eng.ming.20140721.151231
ro.build.date=Mon Jul 21 15:12:51 CST 2014

Comment 41

[Ethan Tseng [:ethan]]

Hi Ming,
I completely forgot this -- RTSP video is not even supported in v1.4!
See bug 980101 - Disable RTSP video support on 1.4.

RTSP video was enabled by default since v2.0.

Comment 42

[Ethan Tseng [:ethan]]

(In reply to Ethan Tseng [:ethan] from comment #41)
> I completely forgot this -- RTSP video is not even supported in v1.4!

Sorry I should recall this fact earlier.
RTSP video is not a released feature in v1.4.
So, would you transfer your RTSP test to v2.0 or v2.1?

Comment 43

[Ming]

(In reply to Ethan Tseng [:ethan] from comment #42)

:) thanks.  I will discuss this with sprd team. 


NI james.

Comment 44

[Rachelle Yang [:ryang][[e-mail]]]

Hi Ethan ,Could you please kindly help provide how to enable RTSP video support on 1.4?
Even RTSP video is not supported, partner would like to enable this by themselves for testing.

Thanks!

Comment 45

[Ethan Tseng [:ethan]]

(In reply to Rachelle Yang [:ryang][[e-mail]] from comment #44)
> Hi Ethan ,Could you please kindly help provide how to enable RTSP video
> support on 1.4?
> Even RTSP video is not supported, partner would like to enable this by
> themselves for testing.

The preference is "media.rtsp.video.enabled".
Just change its value from false to true.

Please note some major issues are expected to happen while testing RTSP on v1.4.
See https://bugzilla.mozilla.org/show_bug.cgi?id=999914#c2 for reference.

Comment 46

[Steve Workman [:sworkman] (INACTIVE)]

Comment on attachment 8464732 [details] [diff] [review]
Fix H263 Assembler

Review of attachment 8464732 [details] [diff] [review]:
-----------------------------------------------------------------

Just some questions - keeping review open.

::: netwerk/protocol/rtsp/rtsp/AH263Assembler.cpp
@@ +104,5 @@
> +    unsigned V = (payloadHeader >> 9) & 1;
> +    unsigned PLEN = (payloadHeader >> 3) & 0x3f;
> +    unsigned PEBIT = payloadHeader & 7;
> +
> +    // V=0

Is there a reference to this in an RFC? If not, why are we asserting this? Please expand the comment a little.

@@ +114,5 @@
> +    }
> +
> +    // RFC 4629, section 5.1
> +    // If PLEN is zero, then PEBIT shall also be zero.
> +    if (PLEN == 0u && PEBIT != 0u) {

This is a little different than the assertions. This will allow PLEN==0 if PEBIT is non-zero - just checking that that is what you want.

@@ +121,5 @@
> +        ALOGW("Packet discarded (PEBIT != 0)");
> +        return MALFORMED_PACKET;
> +    }
> +
> +    size_t skip = V + PLEN + (P ? 0: 2);

Looks like we will not get here if V!=0 - so, V should always be 0 at this point, right?

Comment 47

[Ethan Tseng [:ethan]]

(In reply to Steve Workman [:sworkman] from comment #46)
> Comment on attachment 8464732 [details] [diff] [review]
> Fix H263 Assembler
> Review of attachment 8464732 [details] [diff] [review]:
> -----------------------------------------------------------------
> Just some questions - keeping review open.
> 
> ::: netwerk/protocol/rtsp/rtsp/AH263Assembler.cpp
> @@ +104,5 @@
> > +    unsigned V = (payloadHeader >> 9) & 1;
> > +    // V=0
> Is there a reference to this in an RFC? If not, why are we asserting this?
> Please expand the comment a little.

In Sec. 5.1 General H263+ Payload Header of RFC 4629 (http://tools.ietf.org/pdf/rfc4629.pdf):
V: 1 bit
Indicates the presence of an 8-bit field containing information
for Video Redundancy Coding (VRC), which follows immediately after
the initial 16 bits of the payload header, if present. For syntax
and semantics of that 8-bit VRC field, see Section 5.2.

We don't support VRC for now. Just treat it as malformed packet and discard it.

> @@ +114,5 @@
> > +    }
> > +    // RFC 4629, section 5.1
> > +    // If PLEN is zero, then PEBIT shall also be zero.
> > +    if (PLEN == 0u && PEBIT != 0u) {
> 
> This is a little different than the assertions. This will allow PLEN==0 if
> PEBIT is non-zero - just checking that that is what you want.
> 

Yes, we want to change the assertion.
The previous assertion requires PLEN to be always zero, which is not true in the case of this bug (and that's why the crash happens).
PLEN indicates the length of the extra picture header.
PEBIT indicates the number of bits that shall be ignored in the last byte of the picture header.
According to RFC 4629, if PLEN is zero, then PEBIT shall also be zero.
We want to remove the original restriction of "PLEN=0" and meanwhile add this logic as a check-point.
So, if PLEN is not zero, we don't care whether PEBIT is zero or not.


> @@ +121,5 @@
> > +    size_t skip = V + PLEN + (P ? 0: 2);
> 
> Looks like we will not get here if V!=0 - so, V should always be 0 at this
> point, right?

Right. We should remove V in this statement.

Comment 48

[Ethan Tseng [:ethan]]

Attachment: Fix H263 Assembler

Addressed issues in review comment.

Comment 49

[Steve Workman [:sworkman] (INACTIVE)]

Comment on attachment 8466049 [details] [diff] [review]
Fix H263 Assembler

Review of attachment 8466049 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome! Those extra comments are great. r=me.

Comment 50

[Ethan Tseng [:ethan]]

(In reply to Steve Workman [:sworkman] from comment #49)
> Comment on attachment 8466049 [details] [diff] [review]
> Fix H263 Assembler
> Awesome! Those extra comments are great. r=me.

Yeah! Thanks for your praise. You've made my day today. :)

Comment 51

[Ethan Tseng [:ethan]]

Attachment: Fix H263 Assembler

Refresh comment "r=sworkman".

Comment 52

[Ethan Tseng [:ethan]]

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

Comment 53

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/0b900c9e0940

Comment 54

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0b900c9e0940

Comment 55

[Ryan VanderMeulen [:RyanVM]]

Backed out for Wasabi bustage.
https://hg.mozilla.org/mozilla-central/rev/48609b922d5e

https://tbpl.mozilla.org/php/getParsedLog.php?id=45078817&tree=B2g-Inbound

Comment 56

[Ethan Tseng [:ethan]]

Attachment: Fix H263 Assembler

Fixed compiler error.

Comment 57

[Ethan Tseng [:ethan]]

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

Comment 58

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/1dbd12a7919c

Comment 59

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1dbd12a7919c

Comment 60

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/95cdc6da87c6

Note that per the recent rule change announced in dev-gaia, 1.4 blockers don't have auto-approval to land on v2.0. Please nominate this patch for b2g32 uplift if you feel that it needs to land there as well. Sorry for the inconvenience :(

Comment 61

[Ethan Tseng [:ethan]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #60)
> https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/95cdc6da87c6
> Note that per the recent rule change announced in dev-gaia, 1.4 blockers
> don't have auto-approval to land on v2.0. Please nominate this patch for
> b2g32 uplift if you feel that it needs to land there as well. Sorry for the
> inconvenience :(

Ryan, don't be sorry. I can understand your policy.
Yes, we need it to land on v2.0. I will request for uplift.

Comment 62

[Ethan Tseng [:ethan]]

Comment on attachment 8466569 [details] [diff] [review]
Fix H263 Assembler

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: System crash while playing H263 video over RTSP on B2G
Testing completed: On m-c
Risk to taking this patch (and alternatives if risky): No risk at all
String or UUID changes made by this patch: N/A

Comment 63

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/b1dd24fc6196

Comment 65

[William Hsu [:whsu]]

Attachment: 2014-08-20-18-37-15.png

Hi, Ethan and everyone,

I cannot reproduce this bug on the latest Dolphin build.
Now, it show the warning message. (Attach the screenshot: 2014-08-20-18-37-15.png)

* Build information:
 - Gaia      3e53eb07bf1c2cafeba53812ebe91835089723d3
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/c45814bfd93b
 - BuildID   20140819000201
 - Version   30.0

Comment 66

[Ethan Tseng [:ethan]]

(In reply to William Hsu [:whsu] from comment #65)
> Created attachment 8475823 [details]
> 2014-08-20-18-37-15.png

William,
Thanks for your verification!
But why does the screen shot look different than mine?
Are you playing the video in the B2G browser? The error message is much more pretty than the one generated in the browser.

Comment 67

[William Hsu [:whsu]]

Yes. I also feel it is weird. I follow your steps (Comment 18) to reproduce it.
In general, FxOS should be able to play AAC file on v1.4.
I will discuss with you in-person tomorrow to see if I misunderstand anything.

Thanks for the heads up!
Have a nice day.

Comment 68

[William Hsu [:whsu]]

(In reply to William Hsu [:whsu] from comment #67)
> Yes. I also feel it is weird. I follow your steps (Comment 18) to reproduce
> it.
> In general, FxOS should be able to play AAC file on v1.4.
> I will discuss with you in-person tomorrow to see if I misunderstand
> anything.
> 
> Thanks for the heads up!
> Have a nice day.

Thanks Ethan.
Because we use different branch to test this patch, so we got different result.
v2.0 and newer(master) branch use built-in media player to play RTSP streaming, but v1.4 build uses video app to play RTSP streaming and only have limited format supported.

Comment 69

[Ethan Tseng [:ethan]]

(In reply to William Hsu [:whsu] from comment #68)
> Thanks Ethan.
> Because we use different branch to test this patch, so we got different
> result.
> v2.0 and newer(master) branch use built-in media player to play RTSP
> streaming, but v1.4 build uses video app to play RTSP streaming and only
> have limited format supported.

Yeah. I didn't notice you were testing on v1.4.
Thanks again!

Comment 70

[Paladin]

Attachment: Verify_video.3gp

This bug has been verified to fail on Flame 2.0, 2.1,

Issue steps:
1. Launch Browser
2. open http://116.228.149.59/streaming/video/3gp/video_3gp_index.html
3. Tap on any video file
**. You will see the video play panel appears. But it keeps loading forever

See attachment: Verify_video.3gp and logcat_flame_0149.txt
Reproducing rate: 5/5
Flame2.0  build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141127000203
Version         32.0
Flame2.1 build:
Gaia-Rev        5372b675e018b6aac97d95ff5db8d4bd16addb9b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f34377ae402b
Build-ID        20141127001201
Version         34.0

Comment 71

[Paladin]

Attachment: logcat_flame_0149.txt