ffmpeg/imagemagick features need feature checks

[orlitzky]

We have optional tests that make use of these packages, for example:

sage: a.show(format="webm", iterations=1)  # optional -- ffmpeg

sage: with open(td + 'wave.gif', 'rb') as f: print(b'!\xf9\x04\x08\x14\x00' in f.read())  # optional -- ImageMagick

These tests are run whenever the corresponding "feature" is available, but the feature checks look only for the convert and ffmpeg programs. Both imagemagick and ffmpeg can be built without support for (say) webm files, making the tests above fail. To avoid spurious failures, the features should test for the necessary file format support, likely in the is_functional() method.

CC: @seblabbe

Component: packages: optional

Author: Sébastien Labbé

Branch: 1678c7f

Reviewer: Julian Rüth, Michael Orlitzky

Issue created by migration from https://trac.sagemath.org/ticket/33092

[mkoeppe]

comment:1

I don't think we need to tighten spkg-configure.m4 -- this does not influence the ffmpeg optional tag at all.

These tags are added by runtime Feature checks, not via spkg-configure.m4

[mkoeppe]

comment:2

(this code is from #32174)

[mkoeppe]

comment:4

If these doctests are the only ways that we "need" these features, perhaps it would be enough to just mark them as "not tested"?

[seblabbe]

comment:5

There are like 79 optional imagemagick doctests and 19 optional ffmpeg doctests:

$ git grep "optional -* ImageMagick" | wc
     76     678    7649
$ git grep "optional -* imagemagick" | wc
      3      23     305
$ git grep "optional -* ffmpeg" | wc
     19     151    1895

We can mark two of them as "not tested" if they do not work with all versions of ffmpeg/imagemagick. But I still think it is a good thing to test that the remaining 79 + 19 - 2 doctests still work on all versions of ffmpeg.

My opinion is that either we expect doctests to work and we test them or we expect doctests not to work and we do not test them.

[mkoeppe]

comment:6

Replying to @seblabbe:

We can mark two of them as "not tested" if they do not work with all versions of ffmpeg/imagemagick. But I still think it is a good thing to test that the remaining 79 + 19 - 2 doctests still work on all versions of ffmpeg.

I agree, I meant these 2 failing ones only.

[orlitzky]

comment:7

Those two were only examples. If I turn off all the features that Gentoo lets me turn off on my desktop (some are required by other packages),

sage -t --long --warn-long 97.6 --random-seed=324387541623744523058195579638222709880 src/sage/combinat/crystals/mv_polytopes.py  # 1 doctest failed
sage -t --long --warn-long 97.6 --random-seed=324387541623744523058195579638222709880 src/sage/combinat/tiling.py  # 8 doctests failed
sage -t --long --warn-long 97.6 --random-seed=324387541623744523058195579638222709880 src/sage/combinat/words/paths.py  # 7 doctests failed
sage -t --long --warn-long 97.6 --random-seed=324387541623744523058195579638222709880 src/sage/plot/animate.py  # 66 doctests failed
sage -t --long --warn-long 97.6 --random-seed=324387541623744523058195579638222709880 src/sage/plot/plot3d/tachyon.py  # 3 doctests failed

[seblabbe]

comment:8

I see. Can you provide the log of the failing doctests here or somewhere?

[orlitzky]

Attachment: ffmpeg-imagemagick.log

[orlitzky]

comment:9

Replying to @seblabbe:

I see. Can you provide the log of the failing doctests here or somewhere?

Sure, I just attached it.

[seblabbe]

comment:10

Thank you for the log.

I see the error message convert convert: No decode delegate for this image format (00000000.png). Can you provide what is the output of convert -list Format on this machine? I wonder if the output of such command could be used to check that the feature works in the feature code?

Also I see:

subprocess.CalledProcessError: Command 'cd "/home/mjo/.sage/temp/gantu/27410/dir_787qb00e/"; sage-native-execute convert -dispose Background -delay 35 -loop 3 *.png "/home/mjo/.sage/temp/gantu/27410/dir_m_q6dl3q/my_animation.gif"' returned non-zero exit status 1.

which does not say much about the error. I think the check_call(cmd, shell=True) should be replaced in the method _gif_from_imagemagick by something which returns more info when it fails. I did something like that in the ticket #20343 (see method def pdf). We could do the same here.

[orlitzky]

comment:11

It looks like most of the disabled formats are absent from the list, but SVG and SVGZ (also disabled) appear with mode ---:

$ convert -list Format
   Format L  Mode  Description
--------------------------------------------------------------------------------
      3FR S  r--  Hasselblad Photo RAW
     8BIM P  rw-  Photoshop resource format
 8BIMTEXT P  rw-  Photoshop resource text format
8BIMWTEXT P  rw-  Photoshop resource wide text format
     APP1 P  rw-  Raw application information
 APP1JPEG P  rw-  Raw JPEG binary data
      ART S  rw-  PFS: 1st Publisher
      ARW S  r--  Sony Alpha DSLR RAW
      AVS U  rw+  AVS X image
        B S  rw+  Raw blue samples
      BMP P  rw-  Microsoft Windows bitmap image
     BMP2 P  -w-  Microsoft Windows bitmap image v2
     BMP3 P  -w-  Microsoft Windows bitmap image v3
      BRF S  -w-  BRF ASCII Braille format
        C S  rw+  Raw cyan samples
    CACHE U  ---  Magick Persistent Cache image format
     CALS S  rw-  Continuous Acquisition and Life-cycle Support Type 1 image
            Specified in MIL-R-28002 and MIL-PRF-28002
  CAPTION P  r--  Image caption
      CIN S  rw-  Cineon Image File
     CMYK S  rw+  Raw cyan, magenta, yellow, and black samples
    CMYKA S  rw+  Raw cyan, magenta, yellow, black, and opacity samples
      CR2 S  r--  Canon Photo RAW
      CRW S  r--  Canon Photo RAW
      CUR S  r--  Microsoft Cursor Icon
      CUT S  r--  DR Halo
      DCM S  r--  Digital Imaging and Communications in Medicine image
            See http://medical.nema.org/ for information on DICOM.
      DCR S  r--  Kodak Photo RAW
      DCX S  rw+  ZSoft IBM PC multi-page Paintbrush
      DNG S  r--  Adobe Digital Negative
      DPX P  rw-  SMPTE 268M-2003 (DPX 2.0)
            See http://www.smtpe.org/ for information on DPX.
     EPDF P  rw-  Encapsulated Portable Document Format
      EPI P  rw-  Adobe Encapsulated PostScript Interchange format
      EPS P  rw-  Adobe Encapsulated PostScript
     EPS2 P  -w-  Adobe Level II Encapsulated PostScript
     EPS3 P  -w+  Adobe Level III Encapsulated PostScript
     EPSF P  rw-  Adobe Encapsulated PostScript
     EPSI P  rw-  Adobe Encapsulated PostScript Interchange format
      ERF S  r--  Epson RAW Format
     EXIF P  rw-  Exif digital camera binary data
      FAX P  rw+  Group 3 FAX (Not TIFF Group3 FAX!)
     FITS S  rw-  Flexible Image Transport System
  FRACTAL S  r--  Plasma fractal image
        G S  rw+  Raw green samples
      GIF P  rw+  CompuServe graphics interchange format (version 89a)
    GIF87 P  rw-  CompuServe graphics interchange format (version 87a)
 GRADIENT P  r--  Gradual passing from one shade to another
     GRAY S  rw+  Raw gray samples
    GRAYA S  rw+  Raw gray samples + alpha
HISTOGRAM P  -w-  Histogram of the image
      HRZ S  r--  HRZ: Slow scan TV
     HTML S  -w-  Hypertext Markup Language and a client-side image map
      ICB S  rw+  Truevision Targa image
      ICC P  rw-  ICC Color Profile
      ICM P  rw-  ICC Color Profile
      ICO S  r--  Microsoft Icon
     ICON S  r--  Microsoft Icon
 IDENTITY P  r--  Hald CLUT identity image
    IMAGE P  r--  GraphicsMagick Embedded Image
     INFO S  -w+  Image descriptive information and statistics
     IPTC P  rw-  IPTC Newsphoto
 IPTCTEXT P  rw-  IPTC Newsphoto text format
IPTCWTEXT P  rw-  IPTC Newsphoto text format
   ISOBRL S  -w-  ISO/TR 11548-1 format
  ISOBRL6 S  -w-  ISO/TR 11548-1 format 6dot
        K S  rw+  Raw black samples
      K25 S  r--  Kodak Photo RAW
      KDC S  r--  Kodak Photo RAW
    LABEL P  r--  Image label
        M S  rw+  Raw magenta samples
      M2V S  -w+  MPEG Video Stream
      MAC S  r--  Mac Paint
      MAP U  rw-  Colormap intensities and indices
      MAT S  rw+  MATLAB Level 4.0-6.0 image formats
    MATTE S  -w+  MATTE raw opacity format
      MEF S  r--  Mamiya Photo RAW
     MIFF P  rw+  Magick Image File Format (GraphicsMagick 1.3.36)
     MONO S  rw-  Bi-level bitmap in least-significant-byte first order
      MPC U  rw+  Magick Persistent Cache image format
     MPEG S  -w+  MPEG Video Stream
      MPG S  -w+  MPEG Video Stream
      MRW S  r--  Minolta Photo RAW
      MTV U  rw+  MTV Raytracing image format
      MVG S  rw-  Magick Vector Graphics
      NEF S  r--  Nikon Electronic Format
     NULL P  rw-  Constant image of uniform color
        O S  rw+  Raw opacity samples
      ORF S  r--  Olympus Photo RAW
      OTB S  rw-  On-the-air bitmap
       P7 S  rw+  Xv thumbnail format
      PAL S  rw-  16bit/pixel interleaved YUV
     PALM U  r--  Palm pixmap
      PAM P  rw+  Portable Arbitrary Map format
      PBM P  rw+  Portable bitmap format (black/white)
      PCD S  rw-  Photo CD
     PCDS S  rw-  Photo CD
      PCL S  -w+  Page Control Language
      PCT S  rw-  Apple Macintosh QuickDraw/PICT
      PCX S  rw-  ZSoft IBM PC Paintbrush
      PDB U  rw+  Palm Database ImageViewer Format
      PDF P  rw+  Portable Document Format
      PEF S  r--  Pentax Electronic File
      PFA P  ---  Postscript Type 1 font (ASCII)
      PFB P  ---  Postscript Type 1 font (binary)
      PGM P  rw+  Portable graymap format (gray scale)
    PICON S  rw-  Personal Icon
     PICT S  rw-  Apple Macintosh QuickDraw/PICT
      PIX S  r--  Alias/Wavefront RLE image format
   PLASMA S  r--  Plasma fractal image
      PNM P  rw+  Portable anymap
      PPM P  rw+  Portable pixmap format (color)
  PREVIEW S  -w-  Show a preview an image enhancement, effect, or f/x
       PS P  rw+  Adobe PostScript
      PS2 P  -w+  Adobe Level II PostScript
      PS3 P  -w+  Adobe Level III PostScript
      PWP U  r--  Seattle Film Works
        R S  rw+  Raw red samples
      RAF S  r--  Fuji Photo RAW
      RAS S  rw+  SUN Rasterfile
      RGB S  rw+  Raw red, green, and blue samples
     RGBA S  rw+  Raw red, green, blue, and matte samples
      RLA U  r--  Alias/Wavefront image
      RLE U  r--  Utah Run length encoded image
      SCT U  r--  Scitex HandShake
      SFW U  r--  Seattle Film Works
      SGI S  rw-  Irix RGB image
    SHTML S  -w-  Hypertext Markup Language and a client-side image map
      SR2 S  r--  Sony Photo RAW
      SRF S  r--  Sony Photo RAW
  STEGANO S  r--  Steganographic image
      SUN S  rw+  SUN Rasterfile
      SVG S  ---  Scalable Vector Graphics
     SVGZ S  ---  Scalable Vector Graphics (ZIP compressed)
     TEXT S  rw+  ASCII Text
      TGA S  rw+  Truevision Targa image
     TILE P  r--  Tile image with a texture
            Use the syntax "-size WIDTHxHEIGHT TILE:imagename" to tile the
            specified tile image over a canvas image of size WIDTHxHEIGHT.
      TIM S  r--  PSX TIM
    TOPOL S  r--  TOPOL X Image
      TTF P  ---  TrueType font
      TXT S  rw+  ASCII Text
     UBRL S  -w-  Unicode Text format
    UBRL6 S  -w-  Unicode Text format 6dot
      UIL U  -w-  X-Motif UIL table
     UYVY S  rw-  16bit/pixel interleaved YUV
      VDA S  rw+  Truevision Targa image
    VICAR S  rw-  VICAR rasterfile format
      VID S  rw+  Visual Image Directory
     VIFF S  rw+  Khoros Visualization image
      VST S  rw+  Truevision Targa image
     WBMP S  rw-  Wireless Bitmap (level 0) image
      WPG U  r--  Word Perfect Graphics
      X3F S  r--  Foveon X3 (Sigma/Polaroid) RAW
      XBM S  rw-  X Windows system bitmap (black/white)
       XC P  r--  Constant image uniform color
      XCF S  r--  GIMP image
      XMP P  rw-  Adobe XML metadata
      XPM S  rw-  X Windows system pixmap (color)
       XV S  rw+  Khoros Visualization image
        Y S  rw+  Raw yellow samples
      YUV S  rw-  CCIR 601 4:1:1 or 4:2:2 (8-bit only)

 Meaning of 'L': P=Primary, S=Stable, U=Unstable

[seblabbe]

comment:12

The content of the second column is not the same in my case:

$ convert -list Format
   Format  Module    Mode  Description
-------------------------------------------------------------------------------
      3FR  DNG       r--   Hasselblad CFV/H3D39II
      3G2  MPEG      r--   Media Container
      3GP  MPEG      r--   Media Container
      AAI* AAI       rw+   AAI Dune image
[...]
   YCbCrA* YCbCr     rw+   Raw Y, Cb, Cr, and alpha samples
      YUV* YUV       rw-   CCIR 601 4:1:1 or 4:2:2

* native blob support
r read support
w write support
+ support for multiple images

Most of the doctests are doing a conversion (bunch of pngs) -> gif.

Do you think something based on

convert -list Format | grep PNG

could be a good test to detect that convert *.png output.gif works?

For info, for me it gives this:

      JNG* PNG       rw-   JPEG Network Graphics
      MNG* PNG       rw+   Multiple-image Network Graphics (libpng 1.6.37)
      PNG* PNG       rw-   Portable Network Graphics (libpng 1.6.37)
           See http://www.libpng.org/ for details about the PNG format.
    PNG00* PNG       rw-   PNG inheriting bit-depth, color-type from original if possible
    PNG24* PNG       rw-   opaque or binary transparent 24-bit RGB (zlib 1.2.11)
    PNG32* PNG       rw-   opaque or transparent 32-bit RGBA
    PNG48* PNG       rw-   opaque or binary transparent 48-bit RGB
    PNG64* PNG       rw-   opaque or transparent 64-bit RGBA
     PNG8* PNG       rw-   8-bit indexed with optional binary transparency

[orlitzky]

comment:13

Replying to @seblabbe:

Most of the doctests are doing a conversion (bunch of pngs) -> gif.

Do you think something based on

convert -list Format | grep PNG

I suppose, but you would need to somehow test for read support of PNGs and write support of GIFs. Personally I would just try to convert a 1x1 pixel from png to gif and see if it works.

[seblabbe]

comment:14

Good idea, I will post a branch soon.

[seblabbe]

Commit: e8dbdf9

[seblabbe]

New commits:

ef31790	33092: adding method is_functional to imagemagick feature
e8dbdf9	33092: print log when an error with convert command occurs in animate.py

[seblabbe]

Branch: u/slabbe/33092

[seblabbe]

Author: Sébastien Labbé

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

020e101

33092: print log when an error with convert command occurs in animate.py

[sagetrac-git]

Changed commit from e8dbdf9 to 020e101

[seblabbe]

comment:17

Needs review!

Also, I would be curious to see the following outputs with the current branch for your machine:

sage: from sage.features.imagemagick import ImageMagick
sage: ImageMagick().is_present()
FeatureTestResult('imagemagick', True)
sage: ImageMagick().is_functional()
FeatureTestResult('imagemagick', True)
sage: ImageMagick().require()

Also this allows to run tests on the 5 failings files and list the skipped doctects:

sage -tp --long --show-skipped src/sage/combinat/crystals/mv_polytopes.py src/sage/combinat/tiling.py src/sage/combinat/words/paths.py src/sage/plot/animate.py src/sage/plot/plot3d/tachyon.py

For comparison, for me, it gives this:

Doctesting 5 files using 8 threads.
sage -t --long --random-seed=292498441106400212647883427867965055382 src/sage/plot/plot3d/tachyon.py
    1 test not run due to known bugs
    0 tests not run because we ran out of time
    [406 tests, 13.74 s]
sage -t --long --random-seed=292498441106400212647883427867965055382 src/sage/combinat/crystals/mv_polytopes.py
    0 tests not run because we ran out of time
    [67 tests, 24.64 s]
sage -t --long --random-seed=292498441106400212647883427867965055382 src/sage/combinat/words/paths.py
    2 internet tests not run
    1 test for not implemented functionality not run
    0 tests not run because we ran out of time
    [521 tests, 64.60 s]
sage -t --long --random-seed=292498441106400212647883427867965055382 src/sage/plot/animate.py
    4 not tested tests not run
    0 tests not run because we ran out of time
    [125 tests, 103.21 s]
sage -t --long --random-seed=292498441106400212647883427867965055382 src/sage/combinat/tiling.py
    5 not tested tests not run
    0 tests not run because we ran out of time
    [489 tests, 211.99 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 212.1 seconds
    cpu time: 271.1 seconds
    cumulative wall time: 418.2 seconds
Features detected for doctesting: ffmpeg,imagemagick

[seblabbe]

comment:19

Most of the errors in the attached log were coming from imagemagick. I wait to get new inputs before touching ffmpeg features.

[orlitzky]

comment:20

I get "not functional",

sage: from sage.features.imagemagick import ImageMagick
sage: ImageMagick().is_present()
FeatureTestResult('imagemagick', True)
sage: ImageMagick().is_functional()
FeatureTestResult('imagemagick', False)
sage: ImageMagick().require()

but it looks like the # optional - ImageMagick tests are still being run. For example,

File "src/sage/plot/animate.py", line 168, in sage.plot.animate.Animation
Failed example:
    a[:5]             # optional -- ImageMagick
Expected:
    Animation with 5 frames
Got:
    Command 
       'sage-native-execute convert -dispose Background -delay 20 -loop 0 *.png "/home/mjo/.sage/temp/gantu/4697/tmp_tlrqhxiz.gif"'
    returned non-zero exit status 1.
    Here is the content of the stderr:convert convert: No decode delegate for this image format (00000000.png).
    Here is the content of the stdout:
    Animation with 5 frames

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

7cb0ea3

33092: in few doctests, only check that GIF8 appears in a generated gif file

[sagetrac-git]

Changed commit from 9287a87 to 7cb0ea3

[seblabbe]

comment:77

Replying to @orlitzky:

I still have the failures in comment:33 with my "normal" imagemagick. I would try b"GIF8" in f.read() rather than looking for a specific binary sequence.

Thanks for suggesting an alternative. I did just that in a commit. I hope I fixed the good lines.

[orlitzky]

comment:78

All passing now, thank you!

I have one last nit: I think class convert(Executable) should be capitalized for PEP8. And this new code is commented-out, but I don't mind if you want to keep it:

+        # Alternate way of raising the error (less verbose)
+        #result.check_returncode()

[sagetrac-git]

Changed commit from 7cb0ea3 to 1678c7f

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

e15197f	33092: capitalize class name to respect PEP8
1678c7f	33092: removed unnecessary comment

[seblabbe]

comment:80

Thanks for catching these. I did 2 commits.

[orlitzky]

comment:81

Thanks again.

[orlitzky]

Changed reviewer from Julian Rüth to Julian Rüth, Michael Orlitzky

[seblabbe]

comment:82

I think this ticket should go to 9.5, but it seems for the 9.5 cycle, there was no period within the early rc candidates which was devoted to merging a bunch of tickets fixing just "defects". I think the 9.5 version would be better if more tickets of type "defect" with positive review were merged.

[orlitzky]

comment:83

Yes, otherwise the 9.5 test suite is going to fail all over the place. Same for the lrslib ticket.

[orlitzky]

comment:84

(and it's a regression, since ffmpeg tests weren't enabled in 9.4 when ffmpeg was disabled)

[vbraun]

Changed branch from u/slabbe/33092 to 1678c7f

[vbraun]

comment:86

Probably an improvement but still broken on the buildbot: followup at #33219

[vbraun]

Changed commit from 1678c7f to none

[seblabbe]

comment:88

Another follow-up to fix an issue observed by Justin on MacOSX needs review at #33231