874897 - B2G Video Player can't identify ogg files when the source width for cropping larger then source image width

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Florin Strugariu [:Bebe]]

STR:
1. Add a .ogg or .mp4 file on the SD card 
2. start the video app
3. Play the files

Actual:
3. No files are found on the SD card

Expected:
Files are found and they can be played

This was found while developing automated tests for the video player app

https://github.com/mozilla/gaia-ui-tests/pull/852

Comment 1

[Florin Strugariu [:Bebe]]

I'm seeing this on both 
V1-train
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/762354023009
Gaia   62221047ba3bf9b346d76457116d3104516b28fe
BuildID 20130521230207
Version 18.0

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/a3a49f8d101e
Gaia   5cf0e708f22da929f3f3799e5e0341f0fde2d4c4
BuildID 20130521230210
Version 18.0

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

13:21 nhirata: try closing the app first as in long tap the home button and then hitting the x.
13:22 nhirata: then relaunching the app and seeing if it's there.
13:22 nhirata: it's something I've been trying to tell people since feb
13:22 nhirata: at least
13:22 nhirata: if not earlier
13:22 nhirata: *shrug*
13:22 nhirata: you can't load files on the device while the app is still open else it won't see it

See also bug 830191.

Comment 3

[Zac C (:zac)]

Naoki, we run the identical steps for OGV and 3GP and Webm videos and the tests pass. the only difference is that the file format is different.

Only issue would be that if they are not actually OGG or MP4 but I don't think Florin would mess that up, he knows his stuff.

Comment 4

[Jason Smith [:jsmith]]

Hmm...this likely is an issue in the device storage area if i were to guess.

cc-ing Dave in case he has some thoughts here.

Comment 5

[Zac C (:zac)]

Also Naoki we push to the device and then we launch Video so that's defintely not the problem.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: test files

Hi Zac, 

I am seeing both mp4 as well as ogv 3gp and webm on the device just fine.  Attached are the test files.  (.flv is not supported)

This is with inari and 1.0.1 w/ today's build 5/22.  FYI, hitting home does not close the video application.  It only puts it in the background which is not enough to refresh the list of videos.

Comment 7

[Zac C (:zac)]

Thanks, bebe will try them out tomorrow.

We're definitely not just hitting the home button. See the test case here:

https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/tests/videoplayer/test_play_mp4_video.py

The only thing we might hit is the HTML cache but prior to this we had the other 3 test cases of ogv, 3gp and webm and they coped fine.

Comment 8

[Bob Silverberg [:bsilverberg]]

Also, we are restarting b2g *and* killing all apps before each test, so I imagine that one, or both, of those tasks would cause the videoplayer app to exit.

Comment 9

[Bob Silverberg [:bsilverberg]]

I can confirm that the ogg file, and nhirata's mp4 file are both working on v1.0.1. Zac is going to check v1-train.

Comment 10

[Zac C (:zac)]

OK so with nhirata's mp4 file this is passing on v1-train.

The OGG file, however, is still failing on v1-train.

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/2f8bf6531f39
Gaia   805ae3df8a50609bf89a659687e473c96b683cd2
BuildID 20130522230206
Version 18.0

Naoki, I would like to edit this bug to be valid for Ogg/v1-train only. Can you confirm this from your side?

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

all 4 that are supported have loaded just fine on my unagi that's on today's v1-train.
FYI more test files can be found here : https://github.com/mozilla/qa-testcase-data

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/2f8bf6531f39
Gaia   805ae3df8a50609bf89a659687e473c96b683cd2
BuildID 20130523070206
Version 18.0

Comment 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Which device are you using?  Maybe it's a device/driver issue?

Comment 13

[Zac C (:zac)]

We're using both Inari and Unagi across our team but in comment #10 I was referring to Inari. Comment #1 is against Unagi.

We encoded this file ourselves. We might have to look a bit more into the validity of the encoding but if we use a file from that repo then we'll just be fitting the test conditions to the result rather than the other way around.

What would happen if the audio track encoding is invalid but the video encoding is valid? Is there a maximum/minimum resolution? Some other conditions we are falling foul of?

Comment 14

[Dave Hylands [:dhylands]]

I noticed that *.ogg is classified both as music and as video. See:
https://mxr.mozilla.org/mozilla-central/source/toolkit/content/devicestorage.properties

*.mp4, *.3gp are also listed in both. Perhaps this is contributing to the problem somehow?

So its possible that the order that the API calls are being made somehow influences this as well.

Comment 15

[Zac C (:zac)]

When we run the automated test there's only ever one media file on the device, but if I recalll the test file we are using does not have OGG encoded audio - only video. Dave how would you consider the validity of this half/half case?

I'll compare the files from pass/fail scenarios tomorrow morning for audio/video codec matches.

Comment 16

[Dave Hylands [:dhylands]]

So, right now I think that devicestorage may just be using the file extension and not looking at the contents.

mediadb though, looks at the metadata in the file, so it's entirely possible that there is some type of mismatch/disagreement going on here.

So I think that getting a precise STR (including device/branch) will help. It would be useful if we can reproduce this manually.

Comment 17

[Florin Strugariu [:Bebe]]

So we are doing something like this in our steps:

1. adb shell stop b2g https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/gaia_test.py#L413
2. removeDir('/data/local/indexedDB') and removeDir('/data/b2g/mozilla') [https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/gaia_test.py#L416] 
3. adb shell start b2g https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/gaia_test.py#L418
4. kill all apps https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/gaia_test.py#L461
5. push the video to 'DCIM/100MZLLA' on the SD card https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/tests/videoplayer/test_play_ogg_video.py#L16
6. start the video app https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/tests/videoplayer/test_play_ogg_video.py#L22
7. check that there is a video in the app

We use the file provided in the issue: 
https://github.com/mozilla/gaia-ui-tests/issues/619
marketing-06-09-2010.ogg

this is the output of ogginfo
ogginfo marketing-06-09-2010.ogg 
Processing file "marketing-06-09-2010.ogg"...

New logical stream (#1, serial: 21f853e9): type theora
New logical stream (#2, serial: 464c4f3f): type vorbis
Theora headers parsed for stream 1, information follows...
Version: 3.2.1
Vendor: Xiph.Org libTheora I 20071025 3 2 1
Width: 360
Height: 240
Total image: 368 by 240, crop offset (4, 0)
Framerate 30/1 (30.00 fps)
Pixel aspect ratio 40:33 (1.212121:1)
Frame aspect 1.818182:1
Colourspace unspecified
Pixel format 4:2:0
Target bitrate: 0 kbps
Nominal quality setting (0-63): 52
Vorbis headers parsed for stream 2, information follows...
Version: 0
Vendor: Xiph.Org libVorbis I 20070622 (1.2.0)
Channels: 1
Rate: 22050

Nominal bitrate: 35.333000 kb/s
Upper bitrate not set
Lower bitrate not set
WARNING: EOS not set on stream 1
Theora stream 1:
	Total data length: 531667 bytes
	Playback length: 0m:20.000s
	Average bitrate: 212.666800 kb/s
WARNING: EOS not set on stream 2
Vorbis stream 2:
	Total data length: 95595 bytes
	Playback length: 0m:19.649s
	Average bitrate: 38.919309 kb/s


running this test on unaghi 
with 
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/c4378ff5d057
Gaia   71c3bcdb758324442a5c72fcd24c09cf219c564c
BuildID 20130523230209
Version 18.0


when we open the video app we get a screen that say there are no videos on the memory card

Comment 18

[Florin Strugariu [:Bebe]]

I aso run the tests with all of the .ogg files in https://github.com/mozilla/qa-testcase-data and all the tests have the same result

Comment 19

[Zac C (:zac)]

I'm going to 'fit' our automated test to the data by using a video that we know works so we can catch a regression rather than nothing. 

The video I'll use is `320x240_theora.ogg` and it does not have an audio channel.

However I did still test a few other OGG videos from that qa-testcase-data repo and they aren't actually shown by the video app so I think this bug is still valid at least to bring our attention back to this at some point in the future.

Comment 20

[Zac C (:zac)]

Actually I've changed my mind on comment #19.

As that file still plays on v1.0.1 then this is still clearly a regression unless somebody shows me that the spec has changed.
More accurately a regression of the ability to play the Ogg Vorbis audio codec.

Comment 21

[Aaron Train [:aaronmt]]

Still seeing this on master (07/03)

Comment 22

[Marco Chen [:mchen]]

Hi all,

I tested the ogg video file on master build (09/30) and I can't reproduce.
Could reporter confirm this? Thanks.

Comment 23

[Jason Smith [:jsmith]]

(In reply to Marco Chen [:mchen] (PTO, 09/16, 09/18~09/22) from comment #22)
> Hi all,
> 
> I tested the ogg video file on master build (09/30) and I can't reproduce.
> Could reporter confirm this? Thanks.

Bebe - Can you check if this still reproduces for you?

Comment 24

[Florin Strugariu [:Bebe]]

Our test is still failing for the .ogg file:

We push to the device: 
https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/resources/VID_0001.ogg


The app says that we should load videos to the memory card.
In the adb shell I can find the video where we push it:
root@android:/sdcard/DCIM/100MZLLA # ls
VID_0001.ogg

In the adb logcat I see:

E/GeckoConsole(  582): Content JS ERROR at app://video.gaiamobile.org/gaia_build_defer_index.js:415 in captureFrame: Exception in captureFrame: [Exception... "Index or size is negative or greater than the allowed amount"  code: "1" nsresult: "0x80530001 (IndexSizeError)"  location: "<unknown>"] 
E/GeckoConsole(  582): Content JS WARN at app://video.gaiamobile.org/gaia_build_defer_index.js:407 in fail: Seek failed while creating thumbnail for /sdcard/DCIM/100MZLLA/VID_0001.ogg . Ignoring corrupt file.

When the app starts.

mchen: Any ideas why do we see that error?

Reproduced on:
Gecko  http://hg.mozilla.org/mozilla-central/rev/5a49762ee832
Gaia  e7c011371aa6af696033d4b867fb9152a6985efa
BuildID 20130930071202
Version 27.0a1

BTW the MP4 test is passing

Comment 25

[Marco Chen [:mchen]]

The problem occurs on "ctx.drawImage(player, x, y, w, h - 1, 0, 0, tw, th);" from metadata.js.
It seems that the region between source image to destination scaling image has an error.

Will continue to check it tomorrow.

Comment 26

[Marco Chen [:mchen]]

In gecko side this issue is caused by the line as below and the reason is "the source width for cropping larger then source image width".
http://mxr.mozilla.org/mozilla-central/source/content/canvas/src/CanvasRenderingContext2D.cpp#3038

The strange thing is that 
  1. The resolution of this file is 360 X 240.
  2. The resolution checked in Canvas is 360 X 240.
  3. But the resolution checked in gaia is 436 240.
     https://github.com/mozilla-b2g/gaia/blob/master/apps/video/js/metadata.js#L278

Comment 27

[Zac C (:zac)]

Thanks for researching this Marco.

Do we need to switch component for this bug? Do you know how to fix it?

Comment 28

[Marco Chen [:mchen]]

After looking into oggReader, the 436 is came from pic_width * mPixelAspectRatio.
So the problem will be 
  1. imgSize got by CanvasRenderingContext2D seems to be the picture size.
  2. But resolution got by video app from video tag is VideoContainer size.
  3. So the calculation from VideoContainer size may bigger then image size.

------------------------------

mTheoraState->mInfo::
  version_major = 3 '\003', version_minor = 2 '\002', version_subminor = 1 '\001', 
  frame_width = 368, frame_height = 240, pic_width = 360, pic_height = 240, pic_x = 4, 
  pic_y = 0, fps_numerator = 30, fps_denominator = 1, aspect_numerator = 40, 
  aspect_denominator = 33, colorspace = TH_CS_UNSPECIFIED, pixel_fmt = TH_PF_420, 
  target_bitrate = 0, quality = 52, keyframe_granule_shift = 6

mTheoraState->mPixelAspectRatio::
  1.21212125

Comment 29

[Marco Chen [:mchen]]

Hi Chris,

I am not sure who can help on this case so I needinfo you first.
Could you help this bug or recommend others? Thanks.

Please refer to comment 26 and 28 for the problem.

Comment 30

[Marco Chen [:mchen]]

My suggestion is 
  1. To find a way for getting image/frame size from video tag not only video container size.

because the the video container size will be "frame size * aspect_ratio"
http://mxr.mozilla.org/mozilla-central/source/content/media/ogg/OggReader.cpp#274

Does this make sense?

Comment 31

[Teodosia Pop]

I can reproduce this issue on v1.2 with 3gp videos, ogv and webm videos.

Gecko  http://hg.mozilla.org/releases/mozilla-aurora/rev/decf58f0d596
Gaia  1499c58d0dd052cf7e0208a1d1e86157e702a3e8
BuildID 20131008004009
Version 26.0a2

Comment 32

[Bob Silverberg [:bsilverberg]]

I hate to contradict Teodosia, but I am only seeing this issue with ogg videos on v1.2. The other formats are recognized and playable.

Gecko  http://hg.mozilla.org/releases/mozilla-aurora/rev/decf58f0d596
Gaia   1499c58d0dd052cf7e0208a1d1e86157e702a3e8
BuildID 20131008004009
Version 26.0a2

Comment 33

[cajbir (:cajbir)]

Comment 0 says both MP4 and Ogg files fail to play from the SD card. Is it still the case that MP4 files fail to play, or has the bug morphed into only Ogg files cannot play? Is it all Ogg files or just the specific one identified in  comment 24? Does that file (or any Ogg file) play direct from the web browser rather than the SD card? Does it happen on all B2G devices? If not, what branch/device combo should I use for looking at this?

Comment 34

[Zac C (:zac)]

Hi Chris, it is just the OGG file as described in comment #24. I can't remember how the MP4 came to be fixed but our automated tests work fine for MP4 now.

Yes this fails on other devices we use in Automation (Unagi, Inari, Hamachi).

We have not tried from a web browser. Trying from a native video player works fine but Marco's investigation suggests it is when getting a thumbnail which would explain why it never appears in the Videoplayer.

This definitely used to work for us on v1.0.1 but it regressed for v1.1. As it is so old it will be tricky for us to find a regression range.

Comment 35

[Jason Smith [:jsmith]]

Given that we're not taking patches for 1.1 anymore unless if it's operator blocker, I'd look at Gecko 26 as a branch to use to look into this btw.

Comment 36

[cajbir (:cajbir)]

(In reply to Jason Smith [:jsmith] from comment #35)
> Given that we're not taking patches for 1.1 anymore unless if it's operator
> blocker, I'd look at Gecko 26 as a branch to use to look into this btw.

I'm looking for a branch that will run and reproduce on an Unagi as it's the only device I have. I'm assuming an Unagi default build shows it and I'll try there.

Comment 37

[cajbir (:cajbir)]

Attachment: Test case

I can reproduce on nightly with this test case.

Comment 38

[cajbir (:cajbir)]

Attachment: fix for testcase

This patch fixes the testcase. It adjusts the size of the source width/height based for videos that have a different aspect ratio (Only some Ogg videos as far as I'm aware).

Comment 39

[Jeff Muizelaar [:jrmuizel]]

(In reply to Chris Double (:doublec) from comment #38)
> Created attachment 823701 [details] [diff] [review]
> fix for testcase
> 
> This patch fixes the testcase. It adjusts the size of the source
> width/height based for videos that have a different aspect ratio (Only some
> Ogg videos as far as I'm aware).

Does this match what other browsers do or the spec?

Comment 40

[cajbir (:cajbir)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #39)
>
> Does this match what other browsers do or the spec?

The only other browser that supports Ogg, Chrome, handles this case fine, yes.

Comment 41

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 823701 [details] [diff] [review]
fix for testcase

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

Ok. I assume you'll add a test case for this as well?

Comment 42

[Zac C (:zac)]

Did you guys merge this in? Because our Gaia UI Automation is suddenly saying "unexpected success" !

Comment 43

[cajbir (:cajbir)]

(In reply to Zac C (:zac) from comment #42)
> Did you guys merge this in? Because our Gaia UI Automation is suddenly
> saying "unexpected success" !

Nope, not yet.

Comment 44

[Zac C (:zac)]

(In reply to Chris Double (:doublec) from comment #43)
> (In reply to Zac C (:zac) from comment #42)
> > Did you guys merge this in? Because our Gaia UI Automation is suddenly
> > saying "unexpected success" !
> 
> Nope, not yet.

Well I'm baffled because this test has been passing reliably for 2 days now without any patch?

We are going to enable it in spite of this bug not being closed.

Can you re-try your own test case and/or resolve this bug as it's morphed into a general issue now. Cheers!

Comment 45

[cajbir (:cajbir)]

(In reply to Zac C (:zac) from comment #44)
> Well I'm baffled because this test has been passing reliably for 2 days now
> without any patch?
> 
> We are going to enable it in spite of this bug not being closed.
> 
> Can you re-try your own test case and/or resolve this bug as it's morphed
> into a general issue now. Cheers!

My test case still fails for me without the patch applied on mozilla-central. Are you using a video that has an aspect ratio (ie. VID_0001.ogg)?

Comment 46

[Zac C (:zac)]

(In reply to Chris Double (:doublec) from comment #45)
> (In reply to Zac C (:zac) from comment #44)
> > Well I'm baffled because this test has been passing reliably for 2 days now
> > without any patch?
> > 
> > We are going to enable it in spite of this bug not being closed.
> > 
> > Can you re-try your own test case and/or resolve this bug as it's morphed
> > into a general issue now. Cheers!
> 
> My test case still fails for me without the patch applied on
> mozilla-central. Are you using a video that has an aspect ratio (ie.
> VID_0001.ogg)?

Our test case has not changed since the day we raised this bug! We just kept running it everyday marked with expected to fail until it unexpectedly passed.

I think it was resolved on the Gaia side by this patch:
https://github.com/mozilla-b2g/gaia/pull/12660

But your test case is still relevant so I'll leave the bug open.

Comment 47

[cajbir (:cajbir)]

Attachment: Mochitest

This contains a mochitest for the aspect ratio issue. Test fails without the fix in attachment 823701 [details] [diff] [review] and succeeds with it. The video in the testcase is a shortened version of the one in attachment 752986 [details].

Comment 48

[cajbir (:cajbir)]

Try:  https://tbpl.mozilla.org/?tree=Try&rev=acc2ac6d89f4

Comment 49

[cajbir (:cajbir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d84dca237e37
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d78b5d79128

Comment 50

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

https://hg.mozilla.org/mozilla-central/rev/d84dca237e37
https://hg.mozilla.org/mozilla-central/rev/3d78b5d79128