r65563 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r65562‎ | r65563 | r65564 >
Date:07:49, 27 April 2010
Author:tstarling
Status:deferred (Comments)
Tags:
Comment:
In OggHandler:
* Reverted r65149 again. The passage of time is not improving it.
* Rewrote oggThumb support from scratch.
* Updated documentation, recommend oggThumb with reservations.
* Use multiplication instead of pow() in parseTimeString(), it's more elegant
* Allow seek times to be non-integers, rounded to the nearest 0.1s. The only reason we rounded to the nearest second was because FFmpeg only supported integer seconds. oggThumb allows floating point.
* Provide informative error message if oggThumb is the wrong version
Modified paths:
  • /trunk/extensions/OggHandler/OggHandler.i18n.php (modified) (history)
  • /trunk/extensions/OggHandler/OggHandler.php (modified) (history)
  • /trunk/extensions/OggHandler/OggHandler_body.php (modified) (history)
  • /trunk/extensions/OggHandler/README (modified) (history)

Diff [purge]

Index: trunk/extensions/OggHandler/README
@@ -6,35 +6,48 @@
77
88 require( "$IP/extensions/OggHandler/OggHandler.php" );
99
10 -FFmpeg
 10+oggThumb
 11+--------
1112
12 -We use FFmpeg for creating still images of videos, you will need a copy on your
13 -server.
 13+oggThumb is the best utility currently available for creating still images from
 14+Theora videos. Unfortunately, that's not saying much, it is quite immature.
1415
15 -Some old versions of FFmpeg had a bug which made it extremely slow to seek in
16 -large theora videos in order to generate a thumbnail. This is fixed in the
17 -current version. If you are using an old version of FFmpeg and find that
18 -performance is extremely poor (tens of seconds) to generate thumbnails of
19 -theora videos that are several minutes or more in length, try applying our
20 -ffmpeg-bugfix.diff.
 16+It can be found in the oggvideotools package in some Linux distributions, or
 17+you can download the source from:
2118
22 -Download source: http://ffmpeg.mplayerhq.hu/download.html
23 -About the bug: https://roundup.mplayerhq.hu/roundup/ffmpeg/issue159
 19+http://dev.streamnik.de/oggvideotools.html
2420
25 -Suggested configure line for minimal functionality:
 21+Versions 0.8 (which is the current release at the time of writing) and earlier
 22+are effectively unusable, due to the lack of an output filename parameter. A
 23+fix has been applied in the pre-release version, and is expected to be released
 24+in version 0.9. In the meantime, if you have a Subversion client, you can get
 25+the development version using:
2626
27 -./configure --disable-demuxers --disable-muxers --disable-decoders --disable-encoders \
28 - --disable-ffserver --disable-ffplay --enable-encoder=mjpeg --enable-muxer=mjpeg \
29 - --enable-decoder=theora --enable-demuxer=ogg --disable-network --disable-devices \
30 - --disable-parsers --enable-parser=vp3 --build-suffix=-still
 27+svn co https://oggvideotools.svn.sourceforge.net/svnroot/oggvideotools oggvideotools
3128
32 -Set the FFmpeg binary location with:
 29+For best results, oggThumb should be used with libtheora 1.1 or later.
3330
 31+To enable oggThumb, put the following after the require line in your
 32+LocalSettings.php:
 33+
 34+ $wgOggThumbLocation = '/path/to/oggThumb';
 35+
 36+FFmpeg
 37+------
 38+
 39+FFmpeg can be used to produce still images from Theora videos, but it suffers
 40+from a number of bugs. Some are fixed in the current development version, some
 41+are not. The most serious of these is an inability to create thumbnails where no
 42+keyframe exists between the requested point and the end of the file -- a
 43+situation which is quite common for videos with only one keyframe.
 44+
 45+To enable FFmpeg, set the FFmpeg binary location with:
 46+
3447 $wgFFmpegLocation = '/path/to/ffmpeg';
3548
36 -after the require line in LocalSettings.php. (The default is
37 -'/usr/bin/ffmpeg'.)
 49+after the require line in LocalSettings.php. The default is "/usr/bin/ffmpeg".
 50+If $wgOggThumbLocation is set to something other than false, FFmpeg will not be
 51+used.
3852
3953 Cortado
4054 -------
@@ -53,11 +66,12 @@
5467 PEAR File_Ogg
5568 -------------
5669
57 -I forked the PEAR File_Ogg package and improved it significantly in order to
58 -support this extension. I have now taken over maintainership of File_Ogg and
59 -merged my changes into the latest release. This extension will now work with
60 -either the bundled File_Ogg class, or a File_Ogg package from PEAR with
61 -version 0.3.0 or greater. It is licensed under the LGPL.
 70+OggHandler developer Tim Starling forked the PEAR File_Ogg package and improved
 71+it significantly in order to support this extension. He has now taken over
 72+maintainership of File_Ogg and merged his changes into the latest release.
 73+This extension will now work with either the bundled File_Ogg class, or a
 74+File_Ogg package from PEAR with version 0.3.0 or greater. It is licensed under
 75+the LGPL.
6276
6377 http://pear.php.net/package/File_Ogg
6478
@@ -72,5 +86,3 @@
7387 http://www.everaldo.com/crystal/
7488
7589 They are licensed under the LGPL.
76 -
Index: trunk/extensions/OggHandler/OggHandler.php
@@ -40,28 +40,45 @@
4141
4242 /******************* CONFIGURATION STARTS HERE **********************/
4343
44 -//Location of oggThumb binary
45 -$wgOggThumbLocation = '/usr/bin/oggThumb';
 44+/**
 45+ * The names of the supported video types, as they appear in the PEAR module.
 46+ */
 47+$wgOggVideoTypes = array( 'Theora' );
4648
47 -// Set the supported ogg codecs:
48 -$wgOggVideoTypes = array( 'Theora' );
 49+/**
 50+ * The names of the supported audio types, as they appear in the PEAR module.
 51+ * These types will be described as audio, but only Vorbis is widely supported
 52+ * by the client-side plugins.
 53+ */
4954 $wgOggAudioTypes = array( 'Vorbis', 'Speex', 'FLAC' );
5055
51 -// Location of the FFmpeg binary
 56+/**
 57+ * Location of the FFmpeg binary, or false to use oggThumb. See the notes
 58+ * about thumbnailer choices in the README file.
 59+ */
5260 $wgFFmpegLocation = '/usr/bin/ffmpeg';
5361
54 -// Filename or URL path to the Cortado Java player applet.
55 -//
56 -// If no path is included, the path to this extension's
57 -// directory will be used by default -- this should work
58 -// on most local installations.
59 -//
60 -// You may need to include a full URL here if $wgUploadPath
61 -// specifies a host different from where the wiki pages are
62 -// served -- the applet .jar file must come from the same host
63 -// as the uploaded media files or Java security rules will
64 -// prevent the applet from loading them.
65 -//
 62+/**
 63+ * Location of the oggThumb binary, or false to use FFmpeg. Note that only
 64+ * version 0.9 (expected release in May 2010) or later is supported. See the
 65+ * README file for more details.
 66+ */
 67+$wgOggThumbLocation = false;
 68+
 69+
 70+/**
 71+ * Filename or URL path to the Cortado Java player applet.
 72+ *
 73+ * If no path is included, the path to this extension's
 74+ * directory will be used by default -- this should work
 75+ * on most local installations.
 76+ *
 77+ * You may need to include a full URL here if $wgUploadPath
 78+ * specifies a host different from where the wiki pages are
 79+ * served -- the applet .jar file must come from the same host
 80+ * as the uploaded media files or Java security rules will
 81+ * prevent the applet from loading them.
 82+ */
6683 $wgCortadoJarFile = "cortado-ovt-stripped-0.5.1.jar";
6784
6885 /******************* CONFIGURATION ENDS HERE **********************/
Index: trunk/extensions/OggHandler/OggHandler_body.php
@@ -37,11 +37,12 @@
3838 function parseTimeString( $seekString, $length = false ) {
3939 $parts = explode( ':', $seekString );
4040 $time = 0;
41 - for ( $i = 0; $i < count( $parts ); $i++ ) {
 41+ $multiplier = 1;
 42+ for ( $i = count( $parts ) - 1; $i >= 0; $i--, $multiplier *= 60 ) {
4243 if ( !is_numeric( $parts[$i] ) ) {
4344 return false;
4445 }
45 - $time += intval( $parts[$i] ) * pow( 60, count( $parts ) - $i - 1 );
 46+ $time += $parts[$i] * $multiplier;
4647 }
4748
4849 if ( $time < 0 ) {
@@ -51,6 +52,8 @@
5253 wfDebug( __METHOD__.": specified near-end or past-the-end time {$time}s, using end minus 1s\n" );
5354 $time = $length - 1;
5455 }
 56+ // Round to nearest 0.1s
 57+ $time = round( $time, 1 );
5558 return $time;
5659 }
5760
@@ -58,7 +61,11 @@
5962 if ( isset( $params['thumbtime'] ) ) {
6063 $time = $this->parseTimeString( $params['thumbtime'] );
6164 if ( $time !== false ) {
62 - return 'seek=' . $time;
 65+ $s = sprintf( "%.1f", $time );
 66+ if ( substr( $s, -2 ) == '.0' ) {
 67+ $s = substr( $s, 0, -2 );
 68+ }
 69+ return 'seek=' . $s;
6370 }
6471 }
6572 return 'mid';
@@ -167,7 +174,6 @@
168175 }
169176
170177 function doTransform( $file, $dstPath, $dstUrl, $params, $flags = 0 ) {
171 - global $wgFFmpegLocation;
172178
173179 $width = $params['width'];
174180 $srcWidth = $file->getWidth();
@@ -215,59 +221,46 @@
216222 return new OggVideoDisplay( $file, $targetFileUrl, $dstUrl, $width, $height, $length, $dstPath, $noIcon );
217223 }
218224
219 - $thumbStatus = $this->generateThumb($file, $dstPath, $params, $width, $height);
220 - if( $thumbStatus !== true ) {
221 - return $thumbStatus;
222 - }
223225
224 - return new OggVideoDisplay( $file, $file->getURL(), $dstUrl, $width, $height, $length, $dstPath );
225 - }
226 - /**
227 - * Generate a thumbnail at a specified path
228 - * @param $file The source ogg file
229 - * @param $dstPath The target location for the jpeg
230 - * @param $params The parameters / options for the thumb request
231 - * @param $width The target output width
232 - * @param $height The target output height
233 - */
234 - function generateThumb($file, $dstPath, $params, $width, $height){
235 - global $wgFFmpegLocation, $wgOggThumbLocation;
236 -
237 - $length = $this->getLength( $file );
238 - $thumbtime = false;
 226+ $thumbTime = false;
239227 if ( isset( $params['thumbtime'] ) ) {
240 - $thumbtime = $this->parseTimeString( $params['thumbtime'], $length );
 228+ $thumbTime = $this->parseTimeString( $params['thumbtime'], $length );
241229 }
242 - if ( $thumbtime === false ) {
243 - // If start time param isset use that for the thumb:
244 - if( isset( $params['start'] ) ){
245 - $thumbtime = $this->parseTimeString( $params['start'], $length );
246 - }else{
247 - # Seek to midpoint by default, it tends to be more interesting than the start
248 - $thumbtime = $length / 2;
249 - }
 230+ if ( $thumbTime === false ) {
 231+ # Seek to midpoint by default, it tends to be more interesting than the start
 232+ $thumbTime = $length / 2;
250233 }
 234+
251235 wfMkdirParents( dirname( $dstPath ) );
252236
253 - wfDebug( "Creating video thumbnail at $dstPath\n" );
254 -
255 - // First check for oggThumb
256 - if( $wgOggThumbLocation && is_file( $wgOggThumbLocation ) ){
257 - $cmd = wfEscapeShellArg( $wgOggThumbLocation ) .
258 - ' -t '. intval( $thumbtime ) . ' ' .
259 - ' -n ' . wfEscapeShellArg( $dstPath ) . ' ' .
260 - ' ' . wfEscapeShellArg( $file->getPath() ) . ' 2>&1';
261 - $returnText = wfShellExec( $cmd, $retval );
262 - //check if it was successful or if we should try ffmpeg:
263 - if ( !$this->removeBadFile( $dstPath, $retval ) ) {
264 - return true;
265 - }
 237+ global $wgOggThumbLocation;
 238+ if ( $wgOggThumbLocation !== false ) {
 239+ $status = $this->runOggThumb( $file->getPath(), $dstPath, $thumbTime );
 240+ } else {
 241+ $status = $this->runFFmpeg( $file->getPath(), $dstPath, $thumbTime );
266242 }
 243+ if ( $status === true ) {
 244+ return new OggVideoDisplay( $file, $file->getURL(), $dstUrl, $width, $height,
 245+ $length, $dstPath );
 246+ } else {
 247+ return new MediaTransformError( 'thumbnail_error', $width, $height, $status );
 248+ }
 249+ }
267250
268 - $cmd = wfEscapeShellArg( $wgFFmpegLocation ) .
269 - ' -ss ' . intval( $thumbtime ) . ' ' .
270 - ' -i ' . wfEscapeShellArg( $file->getPath() ) .
271 - # MJPEG, that's the same as JPEG except it's supported by the windows build of ffmpeg
 251+ /**
 252+ * Run FFmpeg to generate a still image from a video file, using a frame close
 253+ * to the given number of seconds from the start.
 254+ *
 255+ * Returns true on success, or an error message on failure.
 256+ */
 257+ function runFFmpeg( $videoPath, $dstPath, $time ) {
 258+ global $wgFFmpegLocation;
 259+ wfDebug( __METHOD__." creating thumbnail at $dstPath\n" );
 260+ $cmd = wfEscapeShellArg( $wgFFmpegLocation ) .
 261+ # FFmpeg only supports integer numbers of seconds
 262+ ' -ss ' . intval( $time ) . ' ' .
 263+ ' -i ' . wfEscapeShellArg( $videoPath ) .
 264+ # MJPEG, that's the same as JPEG except it's supported ffmpeg
272265 # No audio, one frame
273266 ' -f mjpeg -an -vframes 1 ' .
274267 wfEscapeShellArg( $dstPath ) . ' 2>&1';
@@ -276,44 +269,53 @@
277270 $returnText = wfShellExec( $cmd, $retval );
278271
279272 if ( $this->removeBadFile( $dstPath, $retval ) || $retval ) {
280 - #re-attempt encode command on frame time 1 and with mapping (special case for chopped oggs)
281 - $cmd = wfEscapeShellArg( $wgFFmpegLocation ) .
282 - ' -map 0:1 '.
283 - ' -ss 1 ' .
284 - ' -i ' . wfEscapeShellArg( $file->getPath() ) .
285 - ' -f mjpeg -an -vframes 1 ' .
286 - wfEscapeShellArg( $dstPath ) . ' 2>&1';
287 - $retval = 0;
288 - $returnText = wfShellExec( $cmd, $retval );
 273+ // Filter nonsense
 274+ $lines = explode( "\n", str_replace( "\r\n", "\n", $returnText ) );
 275+ if ( substr( $lines[0], 0, 6 ) == 'FFmpeg' ) {
 276+ for ( $i = 1; $i < count( $lines ); $i++ ) {
 277+ if ( substr( $lines[$i], 0, 2 ) != ' ' ) {
 278+ break;
 279+ }
 280+ }
 281+ $lines = array_slice( $lines, $i );
 282+ }
 283+ // Return error message
 284+ return implode( "\n", $lines );
289285 }
 286+ // Success
 287+ return true;
 288+ }
290289
 290+ /**
 291+ * Run oggThumb to generate a still image from a video file, using a frame
 292+ * close to the given number of seconds from the start.
 293+ *
 294+ * Returns true on success, or an error message on failure.
 295+ */
 296+ function runOggThumb( $videoPath, $dstPath, $time ) {
 297+ global $wgOggThumbLocation;
 298+ wfDebug( __METHOD__." creating thumbnail at $dstPath\n" );
 299+ $cmd = wfEscapeShellArg( $wgOggThumbLocation ) .
 300+ ' -t ' . floatval( $time ) .
 301+ ' -n ' . wfEscapeShellArg( $dstPath ) .
 302+ ' ' . wfEscapeShellArg( $videoPath ) . ' 2>&1';
 303+ $retval = 0;
 304+ $returnText = wfShellExec( $cmd, $retval );
 305+
291306 if ( $this->removeBadFile( $dstPath, $retval ) || $retval ) {
292 - #No mapping, time zero. A last ditch attempt.
293 - $cmd = wfEscapeShellArg( $wgFFmpegLocation ) .
294 - ' -ss 0 ' .
295 - ' -i ' . wfEscapeShellArg( $file->getPath() ) .
296 - ' -f mjpeg -an -vframes 1 ' .
297 - wfEscapeShellArg( $dstPath ) . ' 2>&1';
298 -
299 - $retval = 0;
300 - $returnText = wfShellExec( $cmd, $retval );
301 - // If still bad return error:
302 - if ( $this->removeBadFile( $dstPath, $retval ) || $retval ) {
303 - // Filter nonsense
304 - $lines = explode( "\n", str_replace( "\r\n", "\n", $returnText ) );
305 - if ( substr( $lines[0], 0, 6 ) == 'FFmpeg' ) {
306 - for ( $i = 1; $i < count( $lines ); $i++ ) {
307 - if ( substr( $lines[$i], 0, 2 ) != ' ' ) {
308 - break;
309 - }
310 - }
311 - $lines = array_slice( $lines, $i );
312 - }
313 - // Return error box
314 - return new MediaTransformError( 'thumbnail_error', $width, $height, implode( "\n", $lines ) );
 307+ // oggThumb spams both stderr and stdout with useless progress
 308+ // messages, and then often forgets to output anything when
 309+ // something actually does go wrong. So interpreting its output is
 310+ // a challenge.
 311+ $lines = explode( "\n", str_replace( "\r\n", "\n", $returnText ) );
 312+ if ( count( $lines ) > 0
 313+ && preg_match( '/invalid option -- \'n\'$/', $lines[0] ) )
 314+ {
 315+ return wfMsgForContent( 'ogg-oggThumb-version', '0.9' );
 316+ } else {
 317+ return wfMsgForContent( 'ogg-oggThumb-failed' );
315318 }
316319 }
317 - // If we did not return an error return true to continue media thum display
318320 return true;
319321 }
320322
Index: trunk/extensions/OggHandler/OggHandler.i18n.php
@@ -47,6 +47,8 @@
4848 'ogg-dismiss' => 'Close',
4949 'ogg-download' => 'Download file',
5050 'ogg-desc-link' => 'About this file',
 51+ 'ogg-oggThumb-version' => 'OggHandler requires oggThumb version $1 or later.',
 52+ 'ogg-oggThumb-failed' => 'oggThumb failed to create the thumbnail.',
5153 );
5254
5355 /** Message documentation (Message documentation)

Follow-up revisions

RevisionCommit summaryAuthorDate
r65889MFT r65563 (and reverted base revisions r65149 and r65156): oggThumb supporttstarling01:11, 4 May 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r65149* restored basic video thumb support per the already deployed oggThumb ( whil...dale00:17, 17 April 2010

Comments

#Comment by Mdale (talk | contribs)   12:42, 27 April 2010

If there is an error with oggThumb you may want it to fallback to ffmpeg instead of just erroring out ? It may make it slightly harder to trace out thumb rendering errors, but you could always advice to set wgFFmpegLocation to false to exclusively use oggThumb and display its error.


#Comment by Tim Starling (talk | contribs)   20:49, 27 April 2010

Not really, no. Yorn has given me commit access to Ogg Video Tools and I intend to find and fix the bugs in it, not fall back to an equally buggy alternative. There is value in not having FFmpeg installed, for both Wikimedia and non-Wikimedia users. It provides an increase in security due to a reduction in the attack surface.

Status & tagging log