[Mlt-devel] [PATCH 1/3] fix a audiostream leak in producer reopen
Hi,
playing files in a loop mode in melted cause a leak of file descriptors
that seen well from /proc/process id/fd/file no.
the problem comes from avformat's producer re-open code that lost
release of audiostream context.
attached patch fix this behaviour...
--
Maksym Veremeyenko
From 55468ffa298871951ddde9a2ddeb39c7388607c9 Mon Sep 17 00:00:00 2001
From: Maksym Veremeyenko ve...@m1stereo.tv
Date: Sun, 3 Jul 2011 16:32:16 +0300
Subject: [PATCH 1/3] fix a audiostream leak in producer reopen
---
src/modules/avformat/producer_avformat.c | 60 +
1 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/src/modules/avformat/producer_avformat.c b/src/modules/avformat/producer_avformat.c
index b96bf10..314bcd1 100644
--- a/src/modules/avformat/producer_avformat.c
+++ b/src/modules/avformat/producer_avformat.c
@@ -143,6 +143,8 @@ static void producer_set_up_audio( producer_avformat self, mlt_frame frame );
static void apply_properties( void *obj, mlt_properties properties, int flags );
static int video_codec_init( producer_avformat self, int index, mlt_properties properties );
static void get_audio_streams_info( producer_avformat self );
+static void producer_avformat_release( producer_avformat );
+static int audio_codec_init( producer_avformat self, int index, mlt_properties properties );
#ifdef VDPAU
#include vdpau.c
@@ -849,40 +851,18 @@ static void reopen_video( producer_avformat self, mlt_producer producer )
{
mlt_properties properties = MLT_PRODUCER_PROPERTIES( producer );
mlt_service_lock( MLT_PRODUCER_SERVICE( producer ) );
- pthread_mutex_lock( self-audio_mutex );
- if ( self-video_codec )
- {
- avformat_lock();
- avcodec_close( self-video_codec );
- avformat_unlock();
- }
- self-video_codec = NULL;
- if ( self-dummy_context )
- av_close_input_file( self-dummy_context );
- self-dummy_context = NULL;
- if ( self-video_format )
- av_close_input_file( self-video_format );
- self-video_format = NULL;
-
- int audio_index = self-audio_index;
- int video_index = self-video_index;
+ producer_avformat_release( self );
- pthread_mutex_unlock( self-audio_mutex );
- pthread_mutex_unlock( self-video_mutex );
producer_open( self, mlt_service_profile( MLT_PRODUCER_SERVICE(producer) ),
mlt_properties_get( properties, resource ) );
- pthread_mutex_lock( self-video_mutex );
- pthread_mutex_lock( self-audio_mutex );
- self-audio_index = audio_index;
- if ( self-video_format video_index -1 )
- {
- self-video_index = video_index;
- video_codec_init( self, video_index, properties );
- }
+ if ( self-video_format self-video_index -1 )
+ video_codec_init( self, self-video_index, properties );
+
+ if ( self-audio_format self-audio_index -1 )
+ audio_codec_init( self, self-audio_index, properties );
- pthread_mutex_unlock( self-audio_mutex );
mlt_service_unlock( MLT_PRODUCER_SERVICE( producer ) );
}
@@ -2531,38 +2511,45 @@ static int producer_get_frame( mlt_producer producer, mlt_frame_ptr frame, int i
return 0;
}
-static void producer_avformat_close( producer_avformat self )
+static void producer_avformat_release( producer_avformat self )
{
- mlt_log_debug( NULL, producer_avformat_close\n );
-
// Cleanup av contexts
- av_free( self-av_frame );
+ if( self-av_frame )
+ av_freep( self-av_frame );
avformat_lock();
int i;
for ( i = 0; i MAX_AUDIO_STREAMS; i++ )
{
if ( self-audio_resample[i] )
audio_resample_close( self-audio_resample[i] );
+ self-audio_resample[i] = NULL;
mlt_pool_release( self-audio_buffer[i] );
- av_free( self-decode_buffer[i] );
+ self-audio_buffer[i] = NULL;
+ av_freep( self-decode_buffer[i] );
if ( self-audio_codec[i] )
avcodec_close( self-audio_codec[i] );
+ self-audio_codec[i] = NULL;
}
if ( self-video_codec )
avcodec_close( self-video_codec );
+ self-video_codec = NULL;
// Close the file
if ( self-dummy_context )
av_close_input_file( self-dummy_context );
+ self-dummy_context = NULL;
if ( self-seekable self-audio_format )
av_close_input_file( self-audio_format );
+ self-audio_format = NULL;
if ( self-video_format )
av_close_input_file( self-video_format );
+ self-video_format = NULL;
avformat_unlock();
#ifdef VDPAU
vdpau_producer_close( self );
#endif
if ( self-image_cache )
mlt_cache_close( self-image_cache );
+ self-image_cache = NULL;
// Cleanup the mutexes
pthread_mutex_destroy( self-audio_mutex );
@@ -2581,6 +2568,13 @@ static void producer_avformat_close( producer_avformat self )
av_free_packet( pkt );
free( pkt );
}
+}
+
+static void producer_avformat_close( producer_avformat self )
+{
+ mlt_log_debug( NULL, producer_avformat_close\n );
+
+ producer_avformat_release( self );
free( self );
}
--
1.7.4.4
--
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a
[Mlt-devel] [PATCH 2/3] possible fix to use new_seek code
Hi, after fixing re-open in avformat producer i realize that new_seek code was never used. latest commit http://mltframework.org/gitweb/mlt.git?p=mltframework.org/mlt.git;a=commitdiff;h=cc781be46c32b555f7675193e34ca74417aa923c enable new_seek only for H264 in TS. May be that wrong? and new_seek code should be enabled by default for all formats except h264/ts? -- Maksym Veremeyenko From 9cdb1e8242c7a0ec521d1253ac654b7e04a92c55 Mon Sep 17 00:00:00 2001 From: Maksym Veremeyenko ve...@m1stereo.tv Date: Sun, 3 Jul 2011 16:39:09 +0300 Subject: [PATCH 2/3] possible fix to use new_seek code --- src/modules/avformat/producer_avformat.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/modules/avformat/producer_avformat.c b/src/modules/avformat/producer_avformat.c index 314bcd1..1144877 100644 --- a/src/modules/avformat/producer_avformat.c +++ b/src/modules/avformat/producer_avformat.c @@ -1296,7 +1296,7 @@ static int producer_get_image( mlt_frame frame, uint8_t **buffer, mlt_image_form // Turn on usage of new seek API and PTS for seeking int use_new_seek = self-seekable - codec_context-codec_id == CODEC_ID_H264 !strcmp( context-iformat-name, mpegts ); + !(codec_context-codec_id == CODEC_ID_H264 !strcmp( context-iformat-name, mpegts )); if ( mlt_properties_get( properties, new_seek ) ) use_new_seek = mlt_properties_get_int( properties, new_seek ); -- 1.7.4.4 -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
[Mlt-devel] [PATCH 3/3] fix newline on position logging
Hi,
after enabling of new_seek code log message about positioning written
without newline. Please find attached patch what fix that.
--
Maksym Veremeyenko
From 2853c96c7ba271cb34fc48968402547d2c0bb76f Mon Sep 17 00:00:00 2001
From: Maksym Veremeyenko ve...@m1stereo.tv
Date: Sun, 3 Jul 2011 16:40:19 +0300
Subject: [PATCH 3/3] fix newline on position logging
---
src/modules/avformat/producer_avformat.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/modules/avformat/producer_avformat.c b/src/modules/avformat/producer_avformat.c
index 1144877..51e19ae 100644
--- a/src/modules/avformat/producer_avformat.c
+++ b/src/modules/avformat/producer_avformat.c
@@ -930,7 +930,7 @@ static int seek_video( producer_avformat self, mlt_position position,
{
timestamp = ( req_position - 0.1 / source_fps ) /
( av_q2d( stream-time_base ) * source_fps );
-mlt_log_verbose( MLT_PRODUCER_SERVICE(producer), pos %PRId64 pts %PRId64 , req_position, timestamp );
+mlt_log_verbose( MLT_PRODUCER_SERVICE(producer), pos %PRId64 pts %PRId64 \n, req_position, timestamp );
if ( self-first_pts 0 )
timestamp += self-first_pts;
else if ( context-start_time != AV_NOPTS_VALUE )
--
1.7.4.4
--
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2___
Mlt-devel mailing list
Mlt-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mlt-devel
Re: [Mlt-devel] [PATCH 2/3] possible fix to use new_seek code
2011/7/3 Maksym Veremeyenko ve...@m1stereo.tv: Hi, after fixing re-open in avformat producer i realize that new_seek code was never used. latest commit http://mltframework.org/gitweb/mlt.git?p=mltframework.org/mlt.git;a=commitdiff;h=cc781be46c32b555f7675193e34ca74417aa923c enable new_seek only for H264 in TS. May be that wrong? and new_seek code should be enabled by default for all formats except h264/ts? This patch is wrong and rejected. My logic is correct: new_seek is only enabled by default for h264/ts as it was specifically added for AVCHD but caused regressions in other formats. -- +-DRD-+ -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
[Mlt-devel] Git: Make libdv sample aspect ratio consistent with profiles.
src/modules/dv/producer_libdv.c | 19 +-- 1 files changed, 17 insertions(+), 2 deletions(-) New commits: commit 30f37b7209c9f2c981113c508321d58ce091619f Author: Dan Dennedy d...@dennedy.org Date: Sun Jul 3 11:40:37 2011 -0700 Make libdv sample aspect ratio consistent with profiles. Patch by: Maksym Veremeyenko -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
Re: [Mlt-devel] [PATCH 1/3] fix a audiostream leak in producer reopen
2011/7/3 Maksym Veremeyenko ve...@m1stereo.tv: Hi, playing files in a loop mode in melted cause a leak of file descriptors that seen well from /proc/process id/fd/file no. the problem comes from avformat's producer re-open code that lost release of audiostream context. attached patch fix this behaviour... This patch is wrong in a couple of ways. It completely alters the locking mechanisms, and I will be surprised if it works under multi-threaded access such as with kdenlive or with parallel consumer. Your attempt to refactor to common code is noble but naive with respect to the locking requirements, which are different depending upon the state of the locks when these similar actions are invoked. Secondly, it breaks the user-selected audio and video tracks because in producer_open() they are reset to their defaults. That is why they were saved before producer_open() and re-applied afterwards. -- +-DRD-+ -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
[Mlt-devel] Git: Add check for null frame.
src/framework/mlt_consumer.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) New commits: commit 313bf8495416ca85a3c1b83790ebf83b5159588d Author: Dan Dennedy d...@dennedy.org Date: Sun Jul 3 12:28:00 2011 -0700 Add check for null frame. -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
[Mlt-devel] Git: Fix file descriptor leak in reopen_video().
src/modules/avformat/producer_avformat.c | 53 -- 1 files changed, 28 insertions(+), 25 deletions(-) New commits: commit f9023d71359af736f2f899c6f118721d0ce301d3 Author: Dan Dennedy d...@dennedy.org Date: Sun Jul 3 12:51:16 2011 -0700 Fix file descriptor leak in reopen_video(). -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
Re: [Mlt-devel] [PATCH 2/3] possible fix to use new_seek code
On Sun, July 3, 2011 9:34 pm, Dan Dennedy wrote: 2011/7/3 Maksym Veremeyenko ve...@m1stereo.tv: Hi, after fixing re-open in avformat producer i realize that new_seek code was never used. latest commit http://mltframework.org/gitweb/mlt.git?p=mltframework.org/mlt.git;a=commitdiff;h=cc781be46c32b555f7675193e34ca74417aa923c enable new_seek only for H264 in TS. May be that wrong? and new_seek code should be enabled by default for all formats except h264/ts? This patch is wrong and rejected. My logic is correct: new_seek is only enabled by default for h264/ts as it was specifically added for AVCHD but caused regressions in other formats. ahh... i did not belived that it only AVCHD targeted... sorry for noise... -- Maksym Veremeyenko -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
Re: [Mlt-devel] [PATCH 1/3] fix a audiostream leak in producer reopen
On Sun, Jul 3, 2011 at 12:48 PM, Maksym Veremeyenko ve...@m1stereo.tv wrote: On Sun, July 3, 2011 10:01 pm, Dan Dennedy wrote: 2011/7/3 Maksym Veremeyenko ve...@m1stereo.tv: Hi, playing files in a loop mode in melted cause a leak of file descriptors that seen well from /proc/process id/fd/file no. the problem comes from avformat's producer re-open code that lost release of audiostream context. attached patch fix this behaviour... This patch is wrong in a couple of ways. It completely alters the locking mechanisms, and I will be surprised if it works under multi-threaded access such as with kdenlive or with parallel consumer. Your attempt to refactor to common code is noble but naive with respect to the locking requirements, which are different depending upon the state of the locks when these similar actions are invoked. Secondly, it breaks the user-selected audio and video tracks because in producer_open() they are reset to their defaults. That is why they were saved before producer_open() and re-applied afterwards. updated patch attached Please test my latest commits. In my analysis and testing while working on these fixes, I found it is not at all safe to close the audio format and codec contexts at this point - think parallel. No offense to you, but I am not too comfortable with other people's hands getting into avformat producer. That is a very intricate piece of work, and there are a lot of special requirements that only I know about. Thank you for trying, though! It will be nice for someone else to become knowledgeable about this code. If you look at the git log, you will see that I have put major effort to refactoring its code periodically in order to tame this wild beast. If you want to contribute something here, I would accept a patch that refactors my recent change that added a take_lock parameter to producer_open() into a new function producer_open_with_lock() that inside calls producer_open() - similar approach to your patch. -- +-DRD-+ -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
[Mlt-devel] Git: Change some verbose messages with new_seek to debug.
src/modules/avformat/producer_avformat.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) New commits: commit 6481ebb58cf2f477d12fcf563903aeea6fe90928 Author: Dan Dennedy d...@dennedy.org Date: Sun Jul 3 13:12:26 2011 -0700 Change some verbose messages with new_seek to debug. -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
Re: [Mlt-devel] [PATCH 3/3] fix newline on position logging
2011/7/3 Maksym Veremeyenko ve...@m1stereo.tv: Hi, after enabling of new_seek code log message about positioning written without newline. Please find attached patch what fix that. Thanks, I also changed these to be debug since verbose is a fairly common, and we do not normally spew these messages when not using new_seek. -- +-DRD-+ -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
