Recent regression in FreeMediaType
The changes to how AM_MEDIA_TYPE is handled about 2 weeks ago seems to be a bit broken. Specifically, in FreeMediaType, this code: if (pMediaType->pUnk) { pMediaType->pUnk->vt->Release(pMediaType->pUnk); pMediaType->pUnk = NULL; } (well the 'Release') causes a segfault with CoreAVC (gdb reports pUnk->vt as inaccessible memory). I believe this data has already been released (but pUnk not zeroed) by the time this gets called (MemAllocator_Decommit) though I haven't dug into it much. All I know so far is it didn't used to segfault on termination before the patch was applied. I'll try to collect more info, and I realize that as CoreAVC isn't a supported codec, I'm on my own, but I figured I'd report it nonetheless.
2007/3/14, Alan Nisota <alannisota@gmail.com>:
The changes to how AM_MEDIA_TYPE is handled about 2 weeks ago seems to be a bit broken. Specifically, in FreeMediaType, this code: if (pMediaType->pUnk) { pMediaType->pUnk->vt->Release(pMediaType->pUnk); pMediaType->pUnk = NULL; } (well the 'Release') causes a segfault with CoreAVC (gdb reports pUnk->vt as inaccessible memory). I believe this data has already been released (but pUnk not zeroed) by the time this gets called (MemAllocator_Decommit) though I haven't dug into it much. All I know so far is it didn't used to segfault on termination before the patch was applied.
I'll try to collect more info, and I realize that as CoreAVC isn't a supported codec, I'm on my own, but I figured I'd report it nonetheless.
Perhaps, trouble is in win32.c's expMoCopyMediaType,expMoFreeMediaType and others. They are neither clears pUnk in Free nor copyes it in Copy. Try to add "dest->pUnk=0" into expMoFreeMediaType and check if this will help. -- Regards, Vladimir Voroshilov mailto:voroshil@gmail.com JID: voroshil@jabber.ru ICQ: 95587719
Vladimir Voroshilov wrote:
Perhaps, trouble is in win32.c's expMoCopyMediaType,expMoFreeMediaType and others. They are neither clears pUnk in Free nor copyes it in Copy. Try to add "dest->pUnk=0" into expMoFreeMediaType and check if this will help.
Nope. I tried that (had to add pUnk in the right place in the definition of MY_MEDIA_TYPE of course), but to no avail. Neither of those functions are called at all during shutdown (or any other time) by CoreAVC. The only places where CopyMediaType get called are in the Input and Output pin creation, as well as SetDestFmt (CreateMedia type never gets called) FreeMediaType gets called 4 times, in the 1st 2 cases, I'm not sure what allocated the memory (didn't come from a CreateMediaType or CopyMediaType), the second 2 cases were allocated by CopyMediaType.Only the second call to CloseMediaType (one where I haven't been able to track down the allocation yet) contains a 'pUnk' and at the point at which it is called, pUnk appears to be invalid.
2007/3/15, Alan Nisota <alannisota@gmail.com>:
Vladimir Voroshilov wrote:
Perhaps, trouble is in win32.c's expMoCopyMediaType,expMoFreeMediaType and others. They are neither clears pUnk in Free nor copyes it in Copy. Try to add "dest->pUnk=0" into expMoFreeMediaType and check if this will help.
Nope. I tried that (had to add pUnk in the right place in the definition of MY_MEDIA_TYPE of course), but to no avail. Neither of those functions are called at all during shutdown (or any other time) by CoreAVC. The only places where CopyMediaType get called are in the Input and Output pin creation, as well as SetDestFmt (CreateMedia type never gets called) FreeMediaType gets called 4 times, in the 1st 2 cases, I'm not sure what allocated the memory (didn't come from a CreateMediaType or CopyMediaType), the second 2 cases were allocated by CopyMediaType.Only the second call to CloseMediaType (one where I haven't been able to track down the allocation yet) contains a 'pUnk' and at the point at which it is called, pUnk appears to be invalid.
i'm not sure that we are talking about the same thing. We have two implementation of CopyMediaType now. one in mediasample.c and one in win32.c static HRESULT WINAPI expMoCopyMediaType(MY_MEDIA_TYPE* dest, const MY_MEDIA_TYPE* src) { if (!dest || !src) return E_POINTER; memcpy(dest, src, sizeof(MY_MEDIA_TYPE)); ^^^^^^ here we will get to pointers to the same object expMoCopyMediaType does neither "pfmt->punk=0" nor "pfmt->pUnk->vt->AddRef(pfmt->pUnk)". Assume: pfmt1=malloc(AM_MEDIA_TYPE) ... pfmt2=expMoCopyMediaType(pfmt1); Code below will work: expMoFreeMediaType(pfmt1); expMoFreeMediaType(pfmt2); Code below will crash: FreeMediaType(pfmt1); FreeMediaType(pfmt2); Bu if we assume: pfmt1=malloc(AM_MEDIA_TYPE) ... pfmt2=CopyMediaType(pfmt1); Code below will work: expMoFreeMediaType(pfmt1); expMoFreeMediaType(pfmt2); Code below will also work: FreeMediaType(pfmt1); FreeMediaType(pfmt2); Try attached patch. -- Regards, Vladimir Voroshilov mailto:voroshil@gmail.com JID: voroshil@jabber.ru ICQ: 95587719
Vladimir Voroshilov wrote:
2007/3/15, Alan Nisota <alannisota@gmail.com>:
Vladimir Voroshilov wrote:
Perhaps, trouble is in win32.c's expMoCopyMediaType,expMoFreeMediaType and others. They are neither clears pUnk in Free nor copyes it in Copy. Try to add "dest->pUnk=0" into expMoFreeMediaType and check if this will help.
Nope. I tried that (had to add pUnk in the right place in the definition of MY_MEDIA_TYPE of course), but to no avail. Neither of those functions are called at all during shutdown (or any other time) by CoreAVC. The only places where CopyMediaType get called are in the Input and Output pin creation, as well as SetDestFmt (CreateMedia type never gets called) FreeMediaType gets called 4 times, in the 1st 2 cases, I'm not sure what allocated the memory (didn't come from a CreateMediaType or CopyMediaType), the second 2 cases were allocated by CopyMediaType.Only the second call to CloseMediaType (one where I haven't been able to track down the allocation yet) contains a 'pUnk' and at the point at which it is called, pUnk appears to be invalid.
i'm not sure that we are talking about the same thing. We have two implementation of CopyMediaType now. one in mediasample.c and one in win32.c
We are talking cross purposes. the expMo*MediaType functions are not used at all by CoreAVC as far as I can tell. The only functions in use are those used to create the input and output pins in the VideoDecoder dshow stuff. So the only interesting bit is the stuff in mediasample.c, as the others aren't being used.
2007/3/15, Alan Nisota <alannisota@gmail.com>:
Vladimir Voroshilov wrote:
2007/3/15, Alan Nisota <alannisota@gmail.com>:
Vladimir Voroshilov wrote:
Perhaps, trouble is in win32.c's
expMoCopyMediaType,expMoFreeMediaType
and
others. They are neither clears pUnk in Free nor copyes it in Copy. Try to add "dest->pUnk=0" into expMoFreeMediaType and check if this will help.
Nope. I tried that (had to add pUnk in the right place in the definition of MY_MEDIA_TYPE of course), but to no avail. Neither of those functions are called at all during shutdown (or any other time) by CoreAVC. The only places where CopyMediaType get called are in the Input and Output pin creation, as well as SetDestFmt (CreateMedia type never gets called) FreeMediaType gets called 4 times, in the 1st 2 cases, I'm not sure what allocated the memory (didn't come from a CreateMediaType or CopyMediaType), the second 2 cases were allocated by CopyMediaType.Only the second call to CloseMediaType (one where I haven't been able to track down the allocation yet) contains a 'pUnk' and at the point at which it is called, pUnk appears to be invalid.
i'm not sure that we are talking about the same thing. We have two implementation of CopyMediaType now. one in mediasample.c and one in win32.c
We are talking cross purposes. the expMo*MediaType functions are not used at all by CoreAVC as far as I can tell. The only functions in use are those used to create the input and output pins in the VideoDecoder dshow stuff. So the only interesting bit is the stuff in mediasample.c, as the others aren't being used.
You are right (and i'm not). expMo*MediaType are used only when codec imports msdmo.dll. I've rechecked code once again and think that non-zero pUnk member can appear only in binary codec (mplayer code always creates structures with pUnk=0, CopyMediaType always calls pUnk->vt->AddRef if punk is non-zero, FreeMediaType sets pUnk to zero after pUnk->vt->Release). So, i 'm afraid, the only solution is to allow memory leak in AM_MEDIA_TYPE (by removing pUnk->vt->Release). I hope, i did miss nothing. -- Regards, Vladimir Voroshilov mailto:voroshil@gmail.com JID: voroshil@jabber.ru ICQ: 95587719
Hi, 2007/3/14, Alan Nisota <alannisota@gmail.com>:
The changes to how AM_MEDIA_TYPE is handled about 2 weeks ago seems to be a bit broken. Specifically, in FreeMediaType, this code: if (pMediaType->pUnk) { pMediaType->pUnk->vt->Release(pMediaType->pUnk); pMediaType->pUnk = NULL; } (well the 'Release') causes a segfault with CoreAVC (gdb reports pUnk->vt as inaccessible memory). I believe this data has already been released (but pUnk not zeroed) by the time this gets called (MemAllocator_Decommit) though I haven't dug into it much. All I know so far is it didn't used to segfault on termination before the patch was applied.
I'll try to collect more info, and I realize that as CoreAVC isn't a supported codec, I'm on my own, but I figured I'd report it nonetheless.
Reproducible with wmv8 too. Disabling the Release statement avoids SIGSEV. Another blocker for RC2? -- Zuxy Beauty is truth, While truth is beauty. PGP KeyID: E8555ED6
2007/4/9, Zuxy Meng <zuxy.meng@gmail.com>:
Hi,
2007/3/14, Alan Nisota <alannisota@gmail.com>:
The changes to how AM_MEDIA_TYPE is handled about 2 weeks ago seems to be a bit broken. Specifically, in FreeMediaType, this code: if (pMediaType->pUnk) { pMediaType->pUnk->vt->Release(pMediaType->pUnk); pMediaType->pUnk = NULL; } (well the 'Release') causes a segfault with CoreAVC (gdb reports pUnk->vt as inaccessible memory). I believe this data has already been released (but pUnk not zeroed) by the time this gets called (MemAllocator_Decommit) though I haven't dug into it much. All I know so far is it didn't used to segfault on termination before the patch was applied.
I'll try to collect more info, and I realize that as CoreAVC isn't a supported codec, I'm on my own, but I figured I'd report it nonetheless.
Reproducible with wmv8 too. Disabling the Release statement avoids SIGSEV. Another blocker for RC2?
Well. In this case we can just comment out Release statement with apropriate note about reason.
-- Zuxy Beauty is truth, While truth is beauty. PGP KeyID: E8555ED6 _______________________________________________ MPlayer-dev-eng mailing list MPlayer-dev-eng@mplayerhq.hu http://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
-- Regards, Vladimir Voroshilov mailto:voroshil@gmail.com JID: voroshil@jabber.ru ICQ: 95587719
HI 2007/4/9, Zuxy Meng <zuxy.meng@gmail.com>:
Reproducible with wmv8 too. Disabling the Release statement avoids SIGSEV. Another blocker for RC2?
I hope i found the reason. Please test r24363 (for wmv8 and CoreAVC, no more crashes with wmv8 for me). -- Regards, Vladimir Voroshilov mailto:voroshil@gmail.com JID: voroshil@gmail.com, voroshil@jabber.ru ICQ: 95587719
participants (3)
-
Alan Nisota -
Vladimir Voroshilov -
Zuxy Meng