Opened 12 years ago
Closed 10 years ago
#11652 closed Patch - Bug Fix (Fixed)
Fix DXVA segfault upon playback exit
Reported by: | Owned by: | dblain | |
---|---|---|---|
Priority: | minor | Milestone: | 0.28 |
Component: | MythTV - Video Playback | Version: | 0.26-fixes |
Severity: | medium | Keywords: | |
Cc: | Ticket locked: | no |
Description
I've found the reason for the crashes on video exit and fixed it: That's because IDirectXVideoDecoderService_CreateSurface creates surface_count + 1 surfaces (1 frontbuffer + surface_count backbuffers) but IDirectXVideoDecoderService_CreateDecoder is created with surface_count buffers.
I also fixed the significant corruption when playback starts and following a skip: That's because of begin_frame in dxva2.c decoder fails. This patch fixes this.
I've attached the two patches for fixes/0.26
Attachments (3)
Change History (10)
Changed 12 years ago by
Attachment: | 0001-DXVA2_stabilization.commit added |
---|
Changed 12 years ago by
Attachment: | 0001-fix_dxva2_segfaulting.commit added |
---|
Changed 12 years ago by
Attachment: | Version.txt added |
---|
comment:1 Changed 12 years ago by
Milestone: | unknown → 0.27 |
---|---|
Owner: | set to stuartm |
Status: | new → accepted |
comment:2 Changed 12 years ago by
Status: | accepted → infoneeded |
---|
comment:3 Changed 12 years ago by
According to Microsoft MSDN The CreateSurface? method also adds reference counts to the surface buffers: "The method fills the array with IDirect3DSurface9 pointers. The caller must release all of the interface pointers. In addition, the front buffer holds a reference count on each of the back buffers. Therefore, the back buffers are never deleted until the front buffer is deleted."
So I eliminated the additional add of reference counts to all the buffers because this would lead to a Memory leak because our code releases this buffers only once.
Additionally I designed the release of this buffers as SafeRelease? which is recommended by Microsoft: "
- Checks whether the pointer is NULL.
- Calls Release if the pointer is not NULL.
- Sets the pointer to NULL.
"
The change in the desired bitstream type to 2 perhaps should removed from this patch. Background of this is also discussed on MSDN: " For DXVA H.264/Mpeg-4 AVC
The value 2 for bConfigBitstreamRaw has the same meaning as the value 1, except as follows: – If bConfigBitstreamRaw is equal to 2, the slice control data structure that is used is the DXVA_Slice_H264_Short data structure. – Otherwise (bConfigBitstreamRaw is not equal to 2), the slice control data structure that is used is the DXVA_Slice_H264_Long data structure. " Type 2 should lead to smoother transitions at seeking in contents.
comment:6 Changed 12 years ago by
Milestone: | 0.27 → 0.28 |
---|---|
Owner: | changed from stuartm to dblain |
Status: | infoneeded → assigned |
I have committed the segfault and memory leak fixes.
I've not applied the 'stabilisation' patch as that really should be included upstream in ffmpeg first. We can always cherry-pick this back to 0.27-fixes/0.27.1 once they've accepted the changes or made their own.
I've also not included the bitstream change as I believe that should get some testing from a dev who has a windows frontend.
comment:7 Changed 10 years ago by
Resolution: | → Fixed |
---|---|
Status: | assigned → closed |
Thank you for your contribution. I'm closing this ticket as fixed as the fix for the crash was committed 15 months ago. Please consider contributing the DVXA patch upstream if you have not done so already.
The segfault patch seems to contain more changes than simply fixing the off-by-one error. Can you explain those, and maybe move them into a separate patch?