Re: [Mlt-devel] [PATCH 1/3] fix a audiostream leak in producer reopen
04.07.11 18:37, Maksym Veremeyenko написав(ла):
03.07.11 23:06, Dan Dennedy написав(ла):
On Sun, Jul 3, 2011 at 12:48 PM, Maksym Veremeyenkove...@m1stereo.tv
wrote:
On Sun, July 3, 2011 10:01 pm, Dan Dennedy wrote:
2011/7/3 Maksym Veremeyenkove...@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.
[...]
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.
i am not sure it it regression or related, but something influent a
decklink output, to be precise, out output. After hour or two i start
receive messages about decklink audio buffer underflow - it wrote less
samples then 1920
seems to me that latest update to avformat producer uncover an audio
preroll problem to decklink consumer.
applying simple patch:
diff --git a/src/modules/decklink/consumer_decklink.cpp
b/src/modules/decklink/consumer_decklink.cpp
index 0fb2361..0179de3 100644
--- a/src/modules/decklink/consumer_decklink.cpp
+++ b/src/modules/decklink/consumer_decklink.cpp
@@ -54,6 +54,7 @@ private:
int m_isKeyer;
IDeckLinkKeyer* m_deckLinkKeyer;
boolm_terminate_on_pause;
+ uint32_tm_acnt;
IDeckLinkDisplayMode* getDisplayMode()
{
@@ -428,6 +429,16 @@ public:
virtual HRESULT STDMETHODCALLTYPE ScheduledFrameCompleted(
IDeckLinkVideoFrame* completedFrame, BMDOutputFrameCompleti
{
+ uint32_t cnt;
+ m_deckLinkOutput-GetBufferedAudioSampleFrameCount(cnt);
+ if(cnt != m_acnt)
+ {
+ mlt_log_verbose( getConsumer(),
+ ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount %d - %d, m_count=%lld\n,
+ m_acnt, cnt, m_count );
+ m_acnt = cnt;
+ };
+
// When a video frame has been released by the API,
schedule another video frame to be output
// ignore handler if frame was flushed
makes very interesting output:
[...]
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 1919 - 2877, m_count=20164
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 725
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 2877 - 3836, m_count=20937
(5) AS-RUN U0 /BNC00975.mov len 748 pos 702
[dv @ 0x80da0a0] Estimating duration from bitrate, this may be inaccurate
[dv @ 0x82ff1c0] Estimating duration from bitrate, this may be inaccurate
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 3836 - 4794, m_count=21685
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 728
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 4794 - 5754, m_count=22458
(5) AS-RUN U0 /BNC00975.mov len 748 pos 705
[dv @ 0x82ff1c0] Estimating duration from bitrate, this may be inaccurate
[dv @ 0x8326820] Estimating duration from bitrate, this may be inaccurate
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 732
[...]
(5) AS-RUN U0 /BNC00975.mov len 748 pos 696
[dv @ 0x80da0a0] Estimating duration from bitrate, this may be inaccurate
[dv @ 0x80d8580] Estimating duration from bitrate, this may be inaccurate
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 5754 - 6715, m_count=29290
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 6715 - 5754, m_count=29291
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 722
for some reason decklink do not flush or drop some audio packet and as
result it overrun after one second samples. i can't definitely find a
reason, but i will look deeper...
--
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/5 Maksym Veremeyenko ve...@m1stereo.tv:
04.07.11 18:37, Maksym Veremeyenko написав(ла):
03.07.11 23:06, Dan Dennedy написав(ла):
On Sun, Jul 3, 2011 at 12:48 PM, Maksym Veremeyenkove...@m1stereo.tv
wrote:
On Sun, July 3, 2011 10:01 pm, Dan Dennedy wrote:
2011/7/3 Maksym Veremeyenkove...@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.
[...]
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.
i am not sure it it regression or related, but something influent a
decklink output, to be precise, out output. After hour or two i start
receive messages about decklink audio buffer underflow - it wrote less
samples then 1920
seems to me that latest update to avformat producer uncover an audio preroll
problem to decklink consumer.
I can think of any recent change that would have affected it. Please
see if you can use Git bisect to determine an offending commit.
applying simple patch:
diff --git a/src/modules/decklink/consumer_decklink.cpp
b/src/modules/decklink/consumer_decklink.cpp
index 0fb2361..0179de3 100644
--- a/src/modules/decklink/consumer_decklink.cpp
+++ b/src/modules/decklink/consumer_decklink.cpp
@@ -54,6 +54,7 @@ private:
int m_isKeyer;
IDeckLinkKeyer* m_deckLinkKeyer;
bool m_terminate_on_pause;
+ uint32_t m_acnt;
IDeckLinkDisplayMode* getDisplayMode()
{
@@ -428,6 +429,16 @@ public:
virtual HRESULT STDMETHODCALLTYPE ScheduledFrameCompleted(
IDeckLinkVideoFrame* completedFrame, BMDOutputFrameCompleti
{
+ uint32_t cnt;
+ m_deckLinkOutput-GetBufferedAudioSampleFrameCount(cnt);
+ if(cnt != m_acnt)
+ {
+ mlt_log_verbose( getConsumer(),
+ ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount %d - %d, m_count=%lld\n,
+ m_acnt, cnt, m_count );
+ m_acnt = cnt;
+ };
+
// When a video frame has been released by the API, schedule
another video frame to be output
// ignore handler if frame was flushed
makes very interesting output:
[...]
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 1919 - 2877, m_count=20164
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 725
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 2877 - 3836, m_count=20937
(5) AS-RUN U0 /BNC00975.mov len 748 pos 702
[dv @ 0x80da0a0] Estimating duration from bitrate, this may be inaccurate
[dv @ 0x82ff1c0] Estimating duration from bitrate, this may be inaccurate
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 3836 - 4794, m_count=21685
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 728
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 4794 - 5754, m_count=22458
(5) AS-RUN U0 /BNC00975.mov len 748 pos 705
[dv @ 0x82ff1c0] Estimating duration from bitrate, this may be inaccurate
[dv @ 0x8326820] Estimating duration from bitrate, this may be inaccurate
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 732
[...]
(5) AS-RUN U0 /BNC00975.mov len 748 pos 696
[dv @ 0x80da0a0] Estimating duration from bitrate, this may be inaccurate
[dv @ 0x80d8580] Estimating duration from bitrate, this may be inaccurate
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 5754 - 6715, m_count=29290
[consumer decklink] ScheduledFrameCompleted:
GetBufferedAudioSampleFrameCount 6715 - 5754, m_count=29291
(5) AS-RUN U0 /tst_E0001531_43.dv len 773 pos 722
for some reason decklink do not flush or drop some audio packet and as
result it overrun after one second samples. i can't definitely find a
reason, but i will look deeper...
--
Maksym Veremeyenko
--
+-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
Re: [Mlt-devel] [PATCH 1/3] fix a audiostream leak in producer reopen
03.07.11 23:06, Dan Dennedy написав(ла): On Sun, Jul 3, 2011 at 12:48 PM, Maksym Veremeyenkove...@m1stereo.tv wrote: On Sun, July 3, 2011 10:01 pm, Dan Dennedy wrote: 2011/7/3 Maksym Veremeyenkove...@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. there are no leaks noticed with latest git, thanks. 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. ok -- 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
03.07.11 23:06, Dan Dennedy написав(ла): On Sun, Jul 3, 2011 at 12:48 PM, Maksym Veremeyenkove...@m1stereo.tv wrote: On Sun, July 3, 2011 10:01 pm, Dan Dennedy wrote: 2011/7/3 Maksym Veremeyenkove...@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. [...] 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. i am not sure it it regression or related, but something influent a decklink output, to be precise, out output. After hour or two i start receive messages about decklink audio buffer underflow - it wrote less samples then 1920 -- 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
[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
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
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
