[XviD-devel] XVid DirectShow decoder filter patch suggestion.

Anton N. Breusov antonz at library.ntu-kpi.kiev.ua
Wed Mar 9 10:49:18 CET 2005


Good day, XVid team!

I'm using XVid DirectShow decoder in our application
(RTS game with some "Cinematic interface" in main menu via DShow and VMR-9),
and found some problems with it, which want to describe here (sorry for my 
English).

Problem description:

XVid Direct Show filter causes crashing of application using it in some
cases. After some investigation I found that it is very picky in it's
CTransformFilter::CheckInputType () function. If anything other than
MEDIATYPE_Video supplied on input Pin, than CXvidDecoder object unloads XVid
decoder DLL library and never loads it again even if after this was supplied
supported stream (because it loaded only once, in constructor) but leaves
pointer to xvid_decore_func intact, and later tries to call this function
(pointing to freed by FreeLibrary() mem) and this causes crashes. Though in
default graphs such behavior works fine (MEDIATYPE_Video type suggested
first) in many players and other applications that builds own graph from
scratch (or semi-manually) such behaviour is incorrect because it's OK when
some types other than supported suggested on Input Pin, Intelligent Connect
depends on this, and filter must not go into unusable state after this.


How to reproduce:

Open GraphEdit (from DirectX SDK utils), manually insert two filters:

"XVid MPEG-4 video decoder" and "File source (Async.)", give some compatible
DivX/XVid AVI video file to File Source filter. Then try to directly connect
this filters (yes, AVI splitter will be inserted between them), after this
try to at least render XVid filter output pin to insert Video Renderer (or
manually place standard "Video renderer" or "Video Mixing Renderer 9"). And
then try to play file -- video window appears and then program crashes in
functions CXvidDecoder::Transform() at call of xvid_decore_func() .This
happens because when we trying to link source filter with XVid decoder, then
DShow Intelligent Connect mechanism tries direct linking first and this link
is rejected by XVid decoder (here library will be freed and pointers become
invalid) and only after this it finds and loads AVI splitter and tries
connecting via it.

Problem persist in XVid v1.1.0 Beta 1 and also was clearly reported some
time ago here:

http://www.xvid.org/modules.php?op=modload&name=phpBB2&file=viewtopic&t=2329

I'm attaching to this message patch against 1.1.0B1 (created by SVN). Hope
it helps and will be intergrated into future releases.

Patch details:

I'm created private function OpenLib() in CXvidDecoder class, now it's
complementary to OpenLib(). In this function I moved most of constructor's
code (only LoadRegistryInfo() was left here), added unconditional call of it
to constructor, and conditional call into CheckInputType (). Also added code
for resetting pointer to functions and DLL handle to NULL (in constructor
and in CloseLib()) and added a few helpful or just paranoid ;-) checks here
and there. After testing seems that it works fine -- we now can probe any
connections to XVid filter without crashes -- so it's fully reusable.

I'm wandering if we really need to Close library and reopen it again, I
think this needed only in case of input pin reconnection, but I'm not really
familliar yet with XVid source code, so decided to do this in more slow, but
"clear" way.

-- 
Anton N. Breusov 'Antonz', AB21-UANIC.
-------------- next part --------------
Index: dshow/src/CXvidDecoder.cpp

===================================================================

--- dshow/src/CXvidDecoder.cpp	(revision 444)

+++ dshow/src/CXvidDecoder.cpp	(working copy)

@@ -203,11 +203,27 @@

 
 #define XVID_DLL_NAME "xvidcore.dll"
 
-CXvidDecoder::CXvidDecoder(LPUNKNOWN punk, HRESULT *phr) :
-    CVideoTransformFilter(NAME("CXvidDecoder"), punk, CLSID_XVID)
+CXvidDecoder::CXvidDecoder(LPUNKNOWN punk, HRESULT *phr)
+: CVideoTransformFilter(NAME("CXvidDecoder"), punk, CLSID_XVID)
+, m_hdll (NULL)
 {
 	DPRINTF("Constructor");
 
+	xvid_decore_func = NULL; // Hmm, some strange errors appearing if I try to initialize...
+	xvid_global_func = NULL; // ...this in constructor's init-list. So, they assigned here.
+
+	LoadRegistryInfo();
+
+	*phr = OpenLib();
+}
+
+HRESULT CXvidDecoder::OpenLib()
+{

+	DPRINTF("OpenLib");
+

+	if (m_hdll != NULL)
+		return E_UNEXPECTED; // Seems, that library already opened.
+

 	xvid_gbl_init_t init;
 	memset(&init, 0, sizeof(init));
 	init.version = XVID_VERSION;
@@ -216,25 +232,34 @@

 	if (m_hdll == NULL) {
 		DPRINTF("dll load failed");
 		MessageBox(0, XVID_DLL_NAME " not found","Error", MB_TOPMOST);
-		return;
+		return E_FAIL;
 	}
 
 	xvid_global_func = (int (__cdecl *)(void *, int, void *, void *))GetProcAddress(m_hdll, "xvid_global");
 	if (xvid_global_func == NULL) {
+		FreeLibrary(m_hdll);
+		m_hdll = NULL;
 		MessageBox(0, "xvid_global() not found", "Error", MB_TOPMOST);
-		return;
+		return E_FAIL;
 	}
 
 	xvid_decore_func = (int (__cdecl *)(void *, int, void *, void *))GetProcAddress(m_hdll, "xvid_decore");
 	if (xvid_decore_func == NULL) {
+		xvid_global_func = NULL;
+		FreeLibrary(m_hdll);
+		m_hdll = NULL;
 		MessageBox(0, "xvid_decore() not found", "Error", MB_TOPMOST);
-		return;
+		return E_FAIL;
 	}
 
 	if (xvid_global_func(0, XVID_GBL_INIT, &init, NULL) < 0)
 	{
+		xvid_global_func = NULL;
+		xvid_decore_func = NULL;
+		FreeLibrary(m_hdll);
+		m_hdll = NULL;
 		MessageBox(0, "xvid_global() failed", "Error", MB_TOPMOST);
-		return;
+		return E_FAIL;
 	}
 
 	memset(&m_create, 0, sizeof(m_create));
@@ -244,8 +269,6 @@

 	memset(&m_frame, 0, sizeof(m_frame));
 	m_frame.version = XVID_VERSION;
 
-	LoadRegistryInfo();
-
 	USE_IYUV = false;
 	USE_YV12 = false;
 	USE_YUY2 = false;
@@ -302,13 +325,16 @@

 		ar_y = 20;
 		break;
 	}
+	
+	return S_OK;
 }
 
 void CXvidDecoder::CloseLib()
 {
-	DPRINTF("Destructor");
+	DPRINTF("CloseLib");
 
-	if (m_create.handle != NULL) {
+	if ((m_create.handle != NULL) && (xvid_decore_func != NULL))
+	{
 		xvid_decore_func(m_create.handle, XVID_DEC_DESTROY, 0, 0);
 		m_create.handle = NULL;
 	}
@@ -317,12 +343,15 @@

 		FreeLibrary(m_hdll);
 		m_hdll = NULL;
 	}
+	xvid_decore_func = NULL;
+	xvid_global_func = NULL;
 }
 
 /* destructor */
 
 CXvidDecoder::~CXvidDecoder()
 {
+	DPRINTF("Destructor");
 	CloseLib();
 }
 
@@ -344,6 +373,13 @@

 		return VFW_E_TYPE_NOT_ACCEPTED;
 	}
 
+	if (m_hdll == NULL)
+	{

+		HRESULT hr = OpenLib();

+		if (FAILED(hr) || (m_hdll == NULL)) // Paranoid checks.

+			return VFW_E_TYPE_NOT_ACCEPTED;

+	}
+
 	if (*mtIn->FormatType() == FORMAT_VideoInfo)
 	{
 		VIDEOINFOHEADER * vih = (VIDEOINFOHEADER *) mtIn->Format();
@@ -710,6 +746,9 @@

 
 	if (m_create.handle == NULL)
 	{
+		if (xvid_decore_func == NULL)
+			return E_FAIL;
+	
 		if (xvid_decore_func(0, XVID_DEC_CREATE, &m_create, 0) < 0)
 		{
             DPRINTF("*** XVID_DEC_CREATE error");
@@ -769,9 +808,10 @@

 	m_frame.output.csp &= ~XVID_CSP_VFLIP;
 	m_frame.output.csp |= rgb_flip^(g_config.nFlipVideo ? XVID_CSP_VFLIP : 0);
 
+	// Paranoid check.
+	if (xvid_decore_func == NULL)
+		return E_FAIL;
 
-
-
 repeat :
 
 	if (pIn->IsPreroll() != S_OK)
Index: dshow/src/CXvidDecoder.h

===================================================================

--- dshow/src/CXvidDecoder.h	(revision 444)

+++ dshow/src/CXvidDecoder.h	(working copy)

@@ -81,6 +81,7 @@

 private :
 
 	HRESULT ChangeColorspace(GUID subtype, GUID formattype, void * format);
+	HRESULT OpenLib();
 	void CloseLib();
 
 	xvid_dec_create_t m_create;


More information about the XviD-devel mailing list