Skip to content

Commit

Permalink
1. updated to compile on windows with VC9. In particular, made c89 co…
Browse files Browse the repository at this point in the history 
…mpatabile and added a config.w32 for normal windows extension builds. 2. Fixed 2 very significant memory leaks during video decoding. 3. Fixed serious frame decode issue for B frames where random end of video frames would get skipped (at least in current ffmpeg implementations see mpenkov/ffmpeg-tutorial#7 and thanks to sobotka for help).  4.  Tried to fix fps and frame count calcuations though don't know if they work across codecs.
  • Loading branch information
@animetrics
animetrics committed Aug 20, 2013
1 parent 32e90b2 commit f929724
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 53 deletions.
36 changes: 36 additions & 0 deletions config.w32
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// $Id$
// vim:ft=javascript

ARG_WITH("ffmpeg", "ffmpeg support", "no");

if(PHP_FFMPEG != "no")
{
if(
CHECK_LIB("avcodec.lib", "ffmpeg", PHP_FFMPEG + "\\lib")
&& CHECK_LIB("avformat.lib", "ffmpeg", PHP_FFMPEG + "\\lib")
&& CHECK_LIB("avutil.lib", "ffmpeg", PHP_FFMPEG + "\\lib")
&& CHECK_LIB("swscale.lib", "ffmpeg", PHP_FFMPEG + "\\lib")
&& CHECK_HEADER_ADD_INCLUDE("libavcodec\\avcodec.h", "CFLAGS_FFMPEG", PHP_FFMPEG + "\\include"))
{
// Will crash in Release mode if you don't have the following compile
// flag

ADD_FLAG("LDFLAGS_FFMPEG", "/OPT:NOREF");

AC_DEFINE("HAVE_LIBGD20", 1);
AC_DEFINE("HAVE_SWSCALER", 1);
AC_DEFINE("HAVE_FFMPEG_PHP", 1);
AC_DEFINE("COMPILE_DL_FFMPEG", 1);
EXTENSION("ffmpeg", "ffmpeg-php.c ffmpeg_movie.c ffmpeg_frame.c ffmpeg_errorhandler.c ffmpeg_tools.c", true);
//
// Use the following instead of above to build the extension into php
// instead of as ext DLL
//
// EXTENSION("ffmpeg", "ffmpeg-php.c ffmpeg_movie.c ffmpeg_frame.c ffmpeg_errorhandler.c ffmpeg_tools.c");
}
else
{
WARNING("ffmpeg not enabled; libraries and headers not found");
}
}

9 changes: 5 additions & 4 deletions ffmpeg-php.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ PHP_MSHUTDOWN_FUNCTION(ffmpeg)
Add an entry for ffmpeg-php support in phpinfo() */
PHP_MINFO_FUNCTION(ffmpeg)
{
AVCodec *next_codec = NULL;
char *m_codec_list = NULL;
long m_codec_list_len = 0;
long m_codec_len = 0;

php_info_print_table_start();
// php_info_print_table_header(2, "ffmpeg-php", "enabled");
php_info_print_table_row(2, "ffmpeg-php version", FFMPEG_PHP_VERSION);
Expand All @@ -169,10 +174,6 @@ PHP_MINFO_FUNCTION(ffmpeg)
#endif

//phpinfo should show the codec list available to aid developers
AVCodec *next_codec = NULL;
char *m_codec_list = NULL;
long m_codec_list_len = 0;
long m_codec_len = 0;
while((next_codec = av_codec_next(next_codec))) {
//go through each codec and add to the list
m_codec_len = (strlen(next_codec->name) + 5);
Expand Down
Empty file modified ffmpeg-phpinfo.php
100755 → 100644
View file
Empty file.
5 changes: 4 additions & 1 deletion ffmpeg_frame.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ int _php_convert_frame(ff_frame_context *ff_frame, int dst_fmt) {
AVFrame *dst_frame;
int result = 0;

TSRMLS_FETCH();

if (!ff_frame->av_frame) {
return -1;
}
Expand Down Expand Up @@ -430,7 +432,8 @@ FFMPEG_PHP_METHOD(ffmpeg_frame, getPresentationTimestamp)

GET_FRAME_RESOURCE(getThis(), ff_frame);

RETURN_DOUBLE((double)ff_frame->pts / AV_TIME_BASE);
//RETURN_DOUBLE((double)ff_frame->pts / AV_TIME_BASE);
RETURN_DOUBLE(ff_frame->pts);
}
/* }}} */

Expand Down
2 changes: 1 addition & 1 deletion ffmpeg_frame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ typedef struct {
int height;
int pixel_format;
int keyframe;
int64_t pts;
double pts;
} ff_frame_context;

ff_frame_context* _php_create_ffmpeg_frame(INTERNAL_FUNCTION_PARAMETERS);
Expand Down
Loading

2 comments on commit f929724

@tony2001
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand it isn't very realistic to expect, but do you happen to have some tests to make sure new version doesn't contain any new issues? I know the code isn't quite beautiful, but it worked before AFAIK. I don't use it myself, but I would like to keep it working.
In other words, do you have any tests? Any reproduce scripts demonstrating the problem you're fixing?

@animetrics
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish I had been versioning my changes from the beginning so that you could pull in individual commits, but it turned into so much more than I expected so I'm sorry for that. I didn't retroactively write unit tests for the library, however I can give you test scripts that manifest the issues but they are also very straightforward to reproduce. The main issues are 2 memory leaks which are one liners to fix. To replicate, all you have to do is rip frames from a video (getFrame()) and monitor the memory. You will see memory increase after each frame and never recover. This is particularly problematic if you are running the TS version of php inside of apache, because memory will not be reclaimed at the end of the script (since php inside of apache runs as a thread, not a child process). The memory leaks are:

  1. There is no av_free(frame) after av_picture_copy, line 1299. One should be added right after that call.
  2. The av_free(frame) line 1260 should be moved inside the loop. This one will only manifest as a memory leak if you skip frames while ripping, e.g. if you call getFrame(1) and then getFrame(3), without calling getFrame(2). The skipped frames will be leaked.

The missing frames at the end of the video is harder to see because you wouldn't necessarily know that the frames were there. For me, it manifested in crazy ways. In particular, I would get a different number of frames when ripping the video, depending on the machine I ran my script on. Also, I would get different pts values. You need only read tutorial05 of the ffmpeg tutorials https://github.com/chelyaev/ffmpeg-tutorial to see that the original pts calculation in this library is incorrect. The issue I linked to in the pull request mpenkov/ffmpeg-tutorial#7 describes the missing frame issue. Since this library is written in C, I had to jump through some hoops in order to not use globals. It's harder for me to give you a script to reproduce this issue because results are random on different machines and I can't guarantee what the outcome would be. I could give you a video which manifests the problem for me, a script to rip the frames, and give you the proper number of frames contained in the video. You could see whether or not all the frames are extracted. E.g. in a video that has 444 frames, I would sometimes get 439 frames, sometimes 441, but never the full 444.

Lastly, I am worried about stability of my missing frame fix, and the fps and num frames fixes. But I am quite confident about the memory leak fixes.

Please sign in to comment.