871438 - [bluetooth] Unable to watch video sent to b2g phone via BT

Status: RESOLVED FIXED

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

Description

[leo.bugzilla.gecko]

GENERAL Defect Description 
1. Title : Unable to watch video sent to b2g phone via BT
2. Precondition : b2g phone1 paired with b2g phone2
3. Tester's Action : 
  1) Try to send video file via BT to phone1 from phone2 
  2) Cancel the receiving file on phone1 
  3) Try to send same video file via BT to phone1 from phone2 again 
  4) file transfer complete
  5) Click the received file on noti bar
4. Detailed Symptom : Can not see video only black screen appears
5. Expected : video should be played without any problems
6.Reproducibility: Frequency Rate : 100%

Dear Mozilla Team,

when file tranfer is canceled and received again, file header seem to changed.
Please check the cancel process of opp.

Thanks.

Comment 1

[Eric Chou [:ericchou] [:echou]]

It looks very similar with Bug 851995. Please confirm if this is a dup. If it is, then please close this as a duplicate issue.

Thank you.

Comment 2

[leo.bugzilla.gecko]

Dear Mozilla Team,

I think that Bug 851995 is not related this.

Because file size is increased a little when a opp cancel, so this symtom seems that unnecessary packet is inserted to header during opp cancel and send.

If you try this action, then it is reproduced easily.

Thanks.

Comment 3

[Eric Chou [:ericchou] [:echou]]

Attachment: Screenshot: after clicking the 'File received' notification item

Comment 4

[Eric Chou [:ericchou] [:echou]]

(In reply to leo.bugzilla.gecko from comment #2)
> Dear Mozilla Team,
> 
> I think that Bug 851995 is not related this.
> 
> Because file size is increased a little when a opp cancel, so this symtom
> seems that unnecessary packet is inserted to header during opp cancel and
> send.
> 
> If you try this action, then it is reproduced easily.
> 
> Thanks.

Hi,

I tried your STR and still couldn't reproduce. Here's the step-by-step I tried:

1. Use my HTC Sensation XL to record a video which is 1249838 bytes long.
2. Send the recorded video to my FFOS device via bluetooth.
3. Accept the request (the file size shown on the dialog is 1.19 MB) from my FFOS device, so the transfer begins.
4. Interrupt the transfer.
5. Send the same video from my HTC device again.
6. The request pops up, and the file size shown on the dialog is still 1.19 MB. Accept the request and the transfer begins.
7. Wait until it finishes, use adb to check the file size. It's 1249838 bytes, which is what I expected.

However, the video could not be opened from tapping the notification on notification bar. I would get below error and the black screen like the attached screenshot.

E/GeckoConsole(  946): [JavaScript Warning: "HTTP "Content-Type" of "application/octet-stream" is not supported. Load of media resource blob:5a9f906d-352f-4826-ba79-a42cd8042084 failed." {file: "app://video.gaiamobile.org/view.html" line: 0}]

I followed the same steps and tested with gecko/mozilla-central:gaia/master, the video could be played without any problems. I'll investigate more tomorrow.

= Build info =

Gecko (b2g18, with patch of bug 872238) :
119383:ad04d81b3eeb
user:        Dave Hylands <[e-mail]>
date:        Tue May 14 10:44:25 2013 -0700
summary:     Bug 871956 - Fix regression introduced by bug 858416. r=kanru, a=leo+

Gaia (v1-train):

commit f4a59b245ea762eea2e9a30fc136229c0fe6f73f
Author: John Ford <[e-mail]>
Date:   Thu May 9 14:41:16 2013 -0700
Bug 870548 - Gaia commit information should have the commit time, not authoring time r=fabrice
(cherry picked from commit c9049cb2dc3e6b28670c5575448ca3c7d3d1cbe5)

Comment 5

[leo.bugzilla.gecko]

Dear Mozilla Team,

Can you try to test the same step with leo and leo device?

Thanks.

Comment 6

[Eric Chou [:ericchou] [:echou]]

(In reply to leo.bugzilla.gecko from comment #5)
> Dear Mozilla Team,
> 
> Can you try to test the same step with leo and leo device?
> 
> Thanks.

I got the same result as I mentioned in comment 4 when testing with my Leo device.

I just talked to Dominic to make sure that our Gaia guys know about the black screen issue. He told me that bug 850941 would fix the black screen problem, and Bug 851995 would fix the 'video app won't be launched' problem. Both bugs are leo+ and ready to land, therefore I suggest that we wait for them being merged and see if this problem will still exist.

Comment 7

[leo.bugzilla.gecko]

Dear Mozilla Team,

Ok, I suspect the bug 850941, so I'll check it and let you know.

Thanks.

Comment 8

[leo.bugzilla.gecko]

Attachment: opp_extran_byte

Comment 9

[leo.bugzilla.gecko]

Dear Mozilla Team,

I applied patch in bug 850941, but black screen is still appeared and transferred file seem to be broken. 

Onclick BT creates MozActivity With the file name and blob data of the  selected file. Blob data is corrupt (adding extra bytes to the header) Hence it is not able to parse the file.

I have attached the screen shot shows the extra bytes.

Please check the opp process.
Thanks.

Comment 10

[[:fabrice] Fabrice Desré]

I think we need the patch in bug 850542 for that to work.

Comment 11

[leo.bugzilla.gecko]

Dear Mozilla Team,

I found the suspect point as below.

When a transfer canceled and again start transfer, sSentFileLength is added mBodySegmentLength in ServerDataHandler() of BluetoothOppManager.cpp.

if (mWaitingForConfirmationFlag) {
      ReceivingFileConfirmation();
      sSentFileLength += mBodySegmentLength;
      return;
    }

Maybe packet size is increased because mBodySegmentLength is not initialized.
So file type seem to be broken and video file cannot play.

I try to fix this as below,

BluetoothOppManager::AfterFirstPut()
{
  mUpdateProgressCounter = 1;
  .........................
  mSuccessFlag = false;
  mBodySegmentLength=0; //init mBodySegmentLength
} 

After this fix, this bug is not reproduced and opp is working well.

Please review this code and let mo know your opinion.
Thanks.

Comment 12

[Eric Chou [:ericchou] [:echou]]

(In reply to leo.bugzilla.gecko from comment #11)
> Dear Mozilla Team,
> 
> I found the suspect point as below.
> 
> When a transfer canceled and again start transfer, sSentFileLength is added
> mBodySegmentLength in ServerDataHandler() of BluetoothOppManager.cpp.
> 
> if (mWaitingForConfirmationFlag) {
>       ReceivingFileConfirmation();
>       sSentFileLength += mBodySegmentLength;
>       return;
>     }
> 
> Maybe packet size is increased because mBodySegmentLength is not initialized.
> So file type seem to be broken and video file cannot play.
> 
> I try to fix this as below,
> 
> BluetoothOppManager::AfterFirstPut()
> {
>   mUpdateProgressCounter = 1;
>   .........................
>   mSuccessFlag = false;
>   mBodySegmentLength=0; //init mBodySegmentLength
> } 
> 
> After this fix, this bug is not reproduced and opp is working well.
> 
> Please review this code and let mo know your opinion.
> Thanks.

mBodySegmentLength should be assigned in ExtractPacketHeaders() unless the PUT packet does not contain header |Body| or |EndOfBody|. Could you check it for me please? Sniffer log (or hcidump ) helps, too.

Comment 13

[Eric Chou [:ericchou] [:echou]]

I found another problem when trying to reproduce this. Bug 875650.

Comment 14

[Eric Chou [:ericchou] [:echou]]

I found that my STR in comment 4 is slightly different from the STR described by reporter in comment 1. I cancel the transfer session from "receiving device", but the reporter cancels from "sending device".

I retried and found a problem which is filed as bug 875650, however, with current codebase, the user should not be able to send files again if he stops the transferring from "sending device". 

ni? reporter since we need more clues. Could you please attach the video of how you reproduce this? Thanks.

Comment 15

[leo.bugzilla.gecko]

Dear Mozilla Team,

Maybe our b2g versin is different with yours.
My device can restart once stop sending.

I attached the reproduced video file.
Please check it.

Thanks.

Comment 16

[leo.bugzilla.gecko]

Attachment: opp_cancel

Comment 17

[Eric Chou [:ericchou] [:echou]]

Thanks, let me try again later.

Comment 18

[Eric Chou [:ericchou] [:echou]]

Hi Leo,

Thanks for the video. It's quite clear and I can confirm the problem.

Like the reporter said in comment 2 and comment 11, when the transferring session was stopped, mBodySegmentLength wouldn't reset to 0. That would impact the next transferring if the first packet of the next transferring doesn't contain header Body or EndOfBody, which would invoke ExtractPacketHeaders() and refresh mBodySegmentLength in the function.

I'll make a patch and get this fixed today. Thanks for all the help.

Comment 19

[Eric Chou [:ericchou] [:echou]]

(In reply to leo.bugzilla.gecko from comment #11)
> Dear Mozilla Team,
> 
> I found the suspect point as below.
> 
> When a transfer canceled and again start transfer, sSentFileLength is added
> mBodySegmentLength in ServerDataHandler() of BluetoothOppManager.cpp.
> 
> if (mWaitingForConfirmationFlag) {
>       ReceivingFileConfirmation();
>       sSentFileLength += mBodySegmentLength;
>       return;
>     }
> 

To be more specific, this is not the right place where the error happened. The error happens in function ConfirmReceivingFile():

  if (aConfirm) {
    StartFileTransfer();
    if (CreateFile()) {
      success = WriteToFile(mBodySegment.get(), mBodySegmentLength);  => here
    }
  }

Comment 20

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: v1: reset mBodySegmentLength when the transfer session starts

Comment 21

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 754677 [details] [diff] [review]
patch 1: v1: reset mBodySegmentLength when the transfer session starts

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

Looks good to me :)

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +745,5 @@
>      mReceivedDataBufferOffset = 0;
>  
>      // When we cancel the transfer, delete the file and notify complemention
>      if (mAbortFlag) {
> +      ReplyToPut(mPutFinalFlag, false);

We still need to update value of |sSentFileLength| here.

Comment 22

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: final: reset mBodySegmentLength when the transfer session starts, r=gyeh

* readd sSentFileLength += mBodySegmentLength after offline discussion with gyeh.

Comment 23

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/projects/birch/rev/aeffa1e56bec

Comment 24

[leo.bugzilla.gecko]

Hi Eric,

This patch is working well on Leo device.
Thanks for your help.

Thanks.

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/aeffa1e56bec

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/24331faa33f9