search

felixge / node-ar-drone

A node.js client for controlling Parrot AR Drone 2.0 quad-copters.
http://nodecopter.com/
MIT License
1.76k stars 428 forks source link

20 instead of 6 images? (AssertionError) #78

Open janpieper opened 10 years ago

janpieper commented 10 years ago

Can someone please tell me, why 6 png images are expected by this test? Using ffmpeg is broken on my maschine because ffmpeg is deprecated and I only get two data events but if I change using ffmpeg to avconv, i get 20 png images (3 broken, 17 with a printer and a cup).

See #60 to find out why I want to use avconv instead of ffmpeg.

Environment

OS: Ubuntu 12.04 Node: 0.8.25 NPM: 1.2.30 node-ar-drone: 031d2e38

Using ffmpeg (deprecated)

$ npm test

> ar-drone@0.2.1 test /home/jan/workspace/node-ar-drone
> node test/run.js

[0:00:01 0 14/17 82.4% node test/integration/video/PngEncoder/test-sample-encode.js]

  assert.js:102
    throw new assert.AssertionError({

  AssertionError: 2 == 6
      at process.<anonymous> (/home/jan/workspace/node-ar-drone/test/integration/video/PngEncoder/test-sample-encode.js:23:10)
      at process.EventEmitter.emit (events.js:96:17)

[0:00:01 1 16/17 100.0% node test/integration/video/TcpVideoStream/test-connection-timeout.js]

Using avconv

Diff: https://gist.github.com/janpieper/7081811 (prototype)

$ npm test

> ar-drone@0.2.1 test /home/jan/workspace/node-ar-drone
> node test/run.js

[0:00:01 0 14/17 82.4% node test/integration/video/PngEncoder/test-sample-encode.js]

  assert.js:102
    throw new assert.AssertionError({
          ^
  AssertionError: 20 == 6
      at process.<anonymous> (/home/jan/workspace/node-ar-drone/test/integration/video/PngEncoder/test-sample-encode.js:23:10)
      at process.EventEmitter.emit (events.js:96:17)

[0:00:01 1 16/17 100.0% node test/integration/video/TcpVideoStream/test-normal-connection.js]
felixge commented 10 years ago

Can someone please tell me, why 6 png images are expected by this test?

I don't remember exactly anymore, but most likely because I got 6 images when I wrote this test, and therefor used it as the value to assert on to detect regressions in the future ; ).

Using ffmpeg is broken on my maschine because ffmpeg is deprecated

Is that true? My understanding is that Ubuntu is switching to avconv, but that doesn't mean ffmpeg itself is deprecated. Afaik both ffmpeg/avconv are actively developed.

i get 20 png images (3 broken, 17 with a printer and a cup).

Printer + Cup sounds good ; ). However, I don't think there should be broken images.

Anyway, you should probably use the PaVe decoder (also in the repo) to find out how many frames should really be inside this fixture. If it's yet another number we may have to approach this test differently.

janpieper commented 10 years ago

Using ffmpeg is broken on my maschine because ffmpeg is deprecated

Is that true? My understanding is that Ubuntu is switching to avconv, but that doesn't mean ffmpeg itself is deprecated. Afaik both ffmpeg/avconv are actively developed.

Calling ffmpeg -version tells me, it is deprecated and I should use avconv instead:

*** THIS PROGRAM IS DEPRECATED ***
This program is only provided for compatibility and will be removed in a future release.
Please use avconv instead.

See https://github.com/felixge/node-ar-drone/issues/60#issuecomment-26706424 for full output.

Anyway, you should probably use the PaVe decoder (also in the repo) to find out how many frames should really be inside this fixture. If it's yet another number we may have to approach this test differently.

Okay, will have a look.

janpieper commented 10 years ago

@felixge Hmmm, have a look at this line: test/unit/video/test-PaVEParser.js#L23... 20 frames. Can you please run node test/integration/video/PngEncoder/test-sample-encode.js on your maschine and verify if this test works for you?

Edit: Please run the test with avconv instead of ffmpeg too.

felixge commented 10 years ago

@janpieper if PaVE says 20 frames and avconv says 20 frames as well, we should prefer avconv.

That being said, the ffmpeg shipping with ubuntu is very old, so a later ffmpeg version may report the correct frames as well.

Calling ffmpeg -version tells me, it is deprecated and I should use avconv instead:

So ... it's not actually ffmpeg telling you this. When you download the debian src pkg for ffmpeg from ubuntu (see http://archive.ubuntu.com/ubuntu/pool/main/liba/libav/libav_0.8.6-1ubuntu2.debian.tar.gz), you'll notice that they are actually patching ffmpeg to print this message. This is quite common with debian/ubunutu packages, but usually done to backport bugfixes, or float build fixes not merged yet by the upstream. In this case it's actually really shitty because an unsuspecting user think ffmpeg is no longer maintained, when in truth it's just the ffmpeg debian pkg (which the log message does not mention).

tl;dr: ffmpeg is not deprecated. I don't feel strongly for either ffmpeg or avconv, but this ubuntu bullshit annoys me.

Can you please run node test/integration/video/PngEncoder/test-sample-encode.js on your maschine and verify if this test works for you? Edit: Please run the test with avconv instead of ffmpeg too.

I don't have time to install avconv, but here is my output for ffmpeg:

$ node test/integration/video/PngEncoder/test-sample-encode.js
# no failure, exit code 0
$ ffmpeg -v
ffmpeg version 2.0 Copyright (c) 2000-2013 the FFmpeg developers
  built on Aug 23 2013 16:27:20 with Apple clang version 4.0 (tags/Apple/clang-421.0.60) (based on LLVM 3.1svn)
  configuration: --prefix=/usr/local/Cellar/ffmpeg/2.0 --enable-shared --enable-pthreads --enable-gpl --enable-version3 --enable-nonfree --enable-hardcoded-tables --enable-avresample --enable-vda --cc=cc --host-cflags= --host-ldflags= --enable-libx264 --enable-libfaac --enable-libmp3lame --enable-libxvid
  libavutil      52. 38.100 / 52. 38.100
  libavcodec     55. 18.102 / 55. 18.102
  libavformat    55. 12.100 / 55. 12.100
  libavdevice    55.  3.100 / 55.  3.100
  libavfilter     3. 79.101 /  3. 79.101
  libavresample   1.  1.  0 /  1.  1.  0
  libswscale      2.  3.100 /  2.  3.100
  libswresample   0. 17.102 /  0. 17.102
  libpostproc    52.  3.100 / 52.  3.100