jonesmGatcom / ossbuild

Automatically exported from code.google.com/p/ossbuild
0 stars 0 forks source link

Direct3D Video Sink #57

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I open this issue to track the development of the plugin.

This morning I have some spare time at work and I'll try to start playing with 
the code. For instance I'd like to make use of the VideoBaseSink and implement 
prerolling.

Original issue reported on code.google.com by ylatuya on 22 Jul 2010 at 9:57

GoogleCodeExporter commented 9 years ago
Strange bug with d3dvideosink.
It doesn't happen with directshowsink nor directdrawsink

Original comment by ylatuya on 23 Jul 2010 at 10:23

Attachments:

GoogleCodeExporter commented 9 years ago
That is certainly interesting. I'm using it inside a java app with no problems.

Original comment by david.g.hoyt on 24 Jul 2010 at 5:27

GoogleCodeExporter commented 9 years ago
It looks more like a GTK issue to me, though. Using the exact same app (no 
recompile), and switching to dshowvideosink/directdrawsink, it works fine?

Original comment by david.g.hoyt on 24 Jul 2010 at 5:28

GoogleCodeExporter commented 9 years ago
I use autovideosink and I switch from one sink to another by leaving only one 
of them in the plugins' folder. It works fine with 
dshowvideosink/directdrawsink.
It's a very weird error because it affects to all the widgets, even dialogs and 
popup windows. The program still works fine, but widgets are not shown properly.

Original comment by ylatuya on 24 Jul 2010 at 6:18

GoogleCodeExporter commented 9 years ago
GTK isn't using direct3d under the covers, is it? (i wouldn't think so...)

Have you inquired on the GTK mailing list/irc about what could possibly be the 
culprit?

Dialogs/popups shouldn't be affected at all. All of my apps run fine. My java 
app using d3dvideosink has dialogs pop up and i display menus right on top of 
the video without a hitch. Very odd indeed...

Original comment by david.g.hoyt on 24 Jul 2010 at 8:07

GoogleCodeExporter commented 9 years ago
Can you attach a copy of your program/installer so i can try it out locally?

Original comment by david.g.hoyt on 24 Jul 2010 at 8:09

GoogleCodeExporter commented 9 years ago
http://ftp.gnome.org/pub/GNOME/binaries/win32/longomatch/LongoMatch-0.15.8-pre2.
exe

Original comment by ylatuya on 24 Jul 2010 at 8:15

GoogleCodeExporter commented 9 years ago
Please see these threads + bug report:

http://lists.cairographics.org/archives/cairo/2009-April/016975.html
http://lists.cairographics.org/archives/cairo/2006-October/008181.html
https://bugs.freedesktop.org/show_bug.cgi?id=7497

I applied the provided patch and longomatch worked normally again. But 
Microsoft does warn against it (see the documentation here: 
http://msdn.microsoft.com/en-us/library/bb172527(v=VS.85).aspx). Output seemed 
fine, though.

Original comment by david.g.hoyt on 25 Jul 2010 at 6:52

Attachments:

GoogleCodeExporter commented 9 years ago
Which version of cairo are you using? The one we compiled? If so, we could 
instead modify cairo (which should have already addressed the issue ages ago, 
though...). I'd rather we did that instead of using potentially dangerous d3d 
tricks.

Original comment by david.g.hoyt on 25 Jul 2010 at 6:53

GoogleCodeExporter commented 9 years ago
Nice catch!
I'm using  GTK and cairo from www.gtk.org. I need to use GTK <= 2.17 because I 
didn't have time to fix a bug in my app with xoverlay and gtk's client-side 
windows. GStreamer overlays the image in the main window instead of the 
widget's window because it's not a native window (ensure_native() should work 
but I haven't tested it yet).
If it's fixed in the cairo side we should go for the cairo solution :)

Original comment by ylatuya on 25 Jul 2010 at 2:11

GoogleCodeExporter commented 9 years ago
Have you tried relinking your app against the new build? Might work better 
these days...

Have you noticed any other bugs w/ d3dvideosink?

Original comment by david.g.hoyt on 25 Sep 2010 at 12:37

GoogleCodeExporter commented 9 years ago
I have fixed a bug in r845. Everything looks good.
I would remove completely the keep-aspect-ratio property. This role is not part 
of the sink and latest version of videoscale already does that. For instance, 
in playbin2, the videoscale element before the videosink has the property 
add-borders=true.

Original comment by ylatuya on 3 Oct 2010 at 7:08

GoogleCodeExporter commented 9 years ago
Hi,

using gst-inspect seems d3dvideosink support raw yuv frame but if I use it 
without the ffmpegcolorspace element before, it doesn't work, for example this 
pipeline work:

gst-launch.exe -v rtspsrc location=.. ! decodebin ! ffmpegcolorspace ! 
d3dvideosink

while this one:

gst-launch.exe -v rtspsrc location=.. ! decodebin ! d3dvideosink

exit with the error:

ERROR:..\..\..\..\..\Source\gst-plugins-bad\sys\d3dvideosink\d3dvideosink.c:1242
:???: code should not be reached

ffmpegcolorspace conversion is cpu expensive, is this the intended behaviuor or 
is this a bug? In my opinion is a sink support raw yuv frame the colorspace 
conversion should be avoided

thanks

Original comment by drakkan1...@gmail.com on 5 Oct 2010 at 6:59

GoogleCodeExporter commented 9 years ago
the decoded frame are I420 and it seems that d3dvideosink doesn't support this 
format, do you plan to add I420 support? 

Original comment by drakkan1...@gmail.com on 5 Oct 2010 at 7:37

GoogleCodeExporter commented 9 years ago
We don't currently have large colorspace support in d3dvideosink just yet (it's 
on the to-do list). Using ffmpegcolorspace is usually a good idea anyway, b/c 
if you don't know the colorspace support ahead of time (e.g. you use 
autovideosink), then it will help ensure that your video can be processed. If 
it's not needed, then it just acts as a pass-through with minimal impact.

For the record, though, I have hit the same snag ("code should not be reached") 
and I'll be looking into it ASAP.

Original comment by david.g.hoyt on 5 Oct 2010 at 4:19

GoogleCodeExporter commented 9 years ago
thanks for your quick answer, my problem is that on linux a gstreamer based 
player is less cpu and memory intensive of other players such as vlc, on 
windows, instead, gstreamer use more resources than vlc and the main difference 
in my pipeline is the ffmpegcolorspace element, thanks for the explaination 
about ffmpegcolorspace I missed that

Original comment by drakkan1...@gmail.com on 5 Oct 2010 at 9:53

GoogleCodeExporter commented 9 years ago
I added support for I420 in r849. Ill add support for rgb tomorrow too.

Original comment by ylatuya on 5 Oct 2010 at 11:29

GoogleCodeExporter commented 9 years ago
Added xRGB support too!!!
I'll work on the different RGB formats tomorrow :D

Original comment by ylatuya on 6 Oct 2010 at 1:14

GoogleCodeExporter commented 9 years ago
Even with ffmpegcolorspace, I haven't noticed much of a performance impact. I 
ran 4 big buck bunny H.264, 854x480 videos at 0% CPU using the latest in the 
repo. My video card has hardware accelerated decoding, however.

ylatuya: I420/RGB support is great. Good job! (c: I have a couple of fixes 
myself that'll need to be merged it seems.

Original comment by david.g.hoyt on 6 Oct 2010 at 7:04

GoogleCodeExporter commented 9 years ago
Hi, I just compiled the last revision (851) but now the video is not show 
correctly, I'm on windows xp

ylatuya: you can test this problem even with the rtsp link I sent to debug the 
h264 crash

david.g.hoyt: I'm testing with an intel card (946GZ, no hw decoding I think) on 
windows xp

Original comment by drakkan1...@gmail.com on 6 Oct 2010 at 7:25

GoogleCodeExporter commented 9 years ago
What pipeline are you using? Can you show us the output with 
GST_DEBUG=*videosink*:5  and the '-v' flag?

Original comment by ylatuya on 6 Oct 2010 at 9:38

GoogleCodeExporter commented 9 years ago
There is probably a stride issue with widths that are not multiple of 8. The 
copy of the input buffer to the memory card still needs to be done by rows 
(coping a stride for each row) and not the full buffer like I did. I'll fix it 
this evening.

Original comment by ylatuya on 6 Oct 2010 at 9:45

GoogleCodeExporter commented 9 years ago
I'm seeing the same thing. What was wrong w/ what was in there before?

Original comment by david.g.hoyt on 6 Oct 2010 at 8:24

GoogleCodeExporter commented 9 years ago
I have fixed it for all of them except for I420. I'll commit it with a FIXME in 
the meanwhile

Original comment by ylatuya on 6 Oct 2010 at 8:34

GoogleCodeExporter commented 9 years ago
I'm stuck with I420. They have a very weird way of exposing it in the frame 
layout, which is not I420 at all. The u and v planes must be written in a whole 
row each one instead of half of the row. I need to investigate further, but I 
couldn't find any documentation. The problem mostly comes with odd widths, 
where I'm having alignment problems.

Original comment by ylatuya on 7 Oct 2010 at 9:01

GoogleCodeExporter commented 9 years ago
I found it! 
http://msdn.microsoft.com/en-us/library/bb970578(v=VS.85).aspx#_420formats
I'll try to finish along the week.

Original comment by ylatuya on 7 Oct 2010 at 9:30

GoogleCodeExporter commented 9 years ago
Have you checked at vlc's code? They select the back buffer format from the 
desktop display mode. 
http://git.videolan.org/?p=vlc.git;a=blob;f=modules/video_output/msw/direct3d.c;
h=4fd89e0336669a44fdbf6bb2940f97d8fe3bde5a;hb=c05fa12539d7423fc0f833118c816eab72
212df6#l495

And they check the colorspace conversion  in this way:
http://git.videolan.org/?p=vlc.git;a=blob;f=modules/video_output/msw/direct3d.c;
h=4fd89e0336669a44fdbf6bb2940f97d8fe3bde5a;hb=c05fa12539d7423fc0f833118c816eab72
212df6#l660 

Original comment by ylatuya on 13 Oct 2010 at 10:34

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I've seen VLC's code and even used it as a basis for some of what I wrote. Mine 
is a bit different in some regards. And that's interesting b/c I thought to do 
the same thing last night -- use YV12 for I420. But then when I played w/ the 
show_frame() function, I couldn't get it to output correctly. The colors were 
wrong (if I remember correctly).

Original comment by david.g.hoyt on 13 Oct 2010 at 11:12

GoogleCodeExporter commented 9 years ago
I just saw your changes and it looks like you found the same solution. Awesome 
-- good job!

Original comment by david.g.hoyt on 13 Oct 2010 at 11:30

GoogleCodeExporter commented 9 years ago
I420 support has been working everywhere I've tried it so far. Are we close to 
a release?

Original comment by david.g.hoyt on 16 Oct 2010 at 1:13

GoogleCodeExporter commented 9 years ago
Attached is a patch that implements the force-aspect-ratio property.

Original comment by raimo.ja...@gmail.com on 25 Nov 2010 at 11:11

Attachments:

GoogleCodeExporter commented 9 years ago
Thank you for the patch. Applied in r928.

Original comment by david.g.hoyt on 29 Nov 2010 at 5:51

GoogleCodeExporter commented 9 years ago
Added navigation support in r929. Please give it a spin. Should work for 
force-aspect-ratio. It might be useful to back port the navigation code to 
directdrawsink (it doesn't seem to work in all cases).

Original comment by david.g.hoyt on 29 Nov 2010 at 7:55

GoogleCodeExporter commented 9 years ago
I think with navigation support finally added, we're poised to truly make this 
the default and get gst upstream to adopt it. Thoughts?

Perhaps the only thing missing is downgrading successfully for autovideosink if 
the client doesn't support d3d. I believe that should be done on the transition 
from NULL -> PAUSED. I'm not entirely sure how to check it, though. And I'm 
hoping it's not gonna turn into a mess where we have to dynamically check for 
the d3d dlls and load modules, get the function addresses, etc.

Original comment by david.g.hoyt on 29 Nov 2010 at 7:59

GoogleCodeExporter commented 9 years ago
Agreed, I think we should now push for getting it into GStreamer. Only thing 
missing is fullscreen mode, but it can be added later.
For installations in which d3d is missing, I would check it in the plugin 
registration code. If d3d is not present, we don't register it. I believe that 
the plugin won't load either if the d3d libraries are missing.

Original comment by ylatuya on 29 Nov 2010 at 9:54

GoogleCodeExporter commented 9 years ago
Setting window handle in playing/paused state doesn't seem to work, because 
swap chain is not recreated. See the attached patch.

Original comment by raimo.ja...@gmail.com on 29 Nov 2010 at 10:06

Attachments:

GoogleCodeExporter commented 9 years ago
To get it compiled under mingw i have to apply the attached patch.

Maybe someone can look at this and check it in if it doesn't break anything...

Thanks,
Thomas

Original comment by la...@gmx.net on 7 Dec 2010 at 6:35

Attachments:

GoogleCodeExporter commented 9 years ago
Thomas, have you included changes to the autotools files (e.g. makefile.am, 
configure.ac, etc.)?

Original comment by david.g.hoyt on 7 Dec 2010 at 9:00

GoogleCodeExporter commented 9 years ago
No, i have used my own simple Makefile and only downloaded the d3dvideosink.* 
to compile it in my normal cross build environment (mingw under ubuntu).

PS: It's the best videosink for me (after hacking/using ddraw and dshow) with 
supports i420 and window changes at playing out of the box. Thank you very much 
for this! Is there a timeline when it will be available in the official 
GStreamer packages?

Original comment by la...@gmx.net on 8 Dec 2010 at 7:17

GoogleCodeExporter commented 9 years ago
Glad that it's working well for you. :) I wrote it hoping that it would be 
better (in terms of stability and performance) than the existing ones and be 
adopted upstream as the default Windows video sink.

I've spoken w/ the gst devs and we need to put it in a git patch, submit it, 
and then get write access to the repo. so we can maintain it. We don't have a 
scheduled timeline for all of that to happen. I've been really busy at work and 
getting ready for a birth (my new son!) so I've had very little time to pursue 
it to the degree that I'd like to. Plus there's a few minor, minor things that 
need to be worked out with it.

If you can get your mingw build integrated into the gst-plugins-bad build and 
then submit the new patch, I'll go ahead and commit that.

Original comment by david.g.hoyt on 9 Dec 2010 at 8:47

GoogleCodeExporter commented 9 years ago
Ok, then the best wishes for you and your family!

If you have some time (in 18 years or so ;-) it would be great to add 
overlay/colorkey support. I really miss the feature in my streaming app to 
output the video as desktop wallpaper.

Original comment by la...@gmx.net on 10 Dec 2010 at 9:37

GoogleCodeExporter commented 9 years ago
lazyt: Your patch (with some minor modifications) was applied in r950. Please 
let me know if the mingw builds work out-of-the-box now.

Original comment by david.g.hoyt on 23 Dec 2010 at 1:05

GoogleCodeExporter commented 9 years ago
When I compile the code in Visual C++, I get a lot of warnings about unused 
labels. The attached patch removes unused labels, fixes some log messages and 
one indentation.

I also tried compiling the code in mingw GCC 4.5.0 in Fedora 14. There is an 
error in function gst_d3dvideosink_hook_proc, apparently LPCWPSTRUCT is not 
defined in mingw. There is PCWPSTRUCT in mingw, I don't know if that could be 
used instead. The variable isn't used, so it could be removed for now.

Original comment by raimo.ja...@gmail.com on 11 Jan 2011 at 1:19

Attachments:

GoogleCodeExporter commented 9 years ago
In Visual C++, I thought we had disabled those specific warnings. We want to 
keep those lines in case they're needed in the future (they've already been 
tested). So at a minimum they would need to be commented out, but not deleted.

LPCWPSTRUCT is defined in mingw-w64. mingw.org, as far as I'm concerned, is 
becoming extinct due to lethargy and hostility towards their user community. At 
any rate, mingw-w64 has it defined in winuser.h as simply a CWPSTRUCT pointer  
(http://msdn.microsoft.com/en-us/library/ms644964(VS.85).aspx). I would suggest 
you submit a patch to the mingw folks to ensure it's defined. Otherwise you 
could do something as simple as -DLPCWPSTRUCT=PCWPSTRUCT. But it won't be 
changed and it might be needed in the future.

Original comment by david.g.hoyt on 11 Jan 2011 at 8:05

GoogleCodeExporter commented 9 years ago
Thanks for the patch, I fixed the misspellings and re-enabled the unused labels 
warning and commented them out in r956. I also commented out the LPCWPSTRUCT 
line, but mingw will still need to be updated because it's anticipated that 
it'll be used in the future. Please let us know if it's compiling better with 
gcc (mingw).

Original comment by david.g.hoyt on 12 Jan 2011 at 8:41

GoogleCodeExporter commented 9 years ago
Thanks. I found one issue, some string arguments work correctly only if UNICODE 
is defined. Attached is a patch that should fix that. RemoveProp was also 
called with the wrong name, see the last change in the patch.

Mingw built plugin seems to be working fine. There's still a lot of gcc 
warnings, I'll send a patch for those later.

LPCWPSTRUCT compiles fine if I add -DLPCWPSTRUCT=PCWPSTRUCT to CFLAGS, so 
that's not a problem.

Original comment by raimo.ja...@gmail.com on 12 Jan 2011 at 10:35

Attachments:

GoogleCodeExporter commented 9 years ago
raimo.jarvi: Thanks again for your patch. The fixes were applied in r958 in a 
slightly different manner (I used TEXT() instead of _T() -- see 
http://blogs.msdn.com/b/oldnewthing/archive/2004/02/12/71851.aspx). Thanks for 
catching the RemoveProp() issue.

Original comment by david.g.hoyt on 13 Jan 2011 at 5:56

GoogleCodeExporter commented 9 years ago
Attached is a patch that fixes some gcc warnings for unused stuff, undeclared 
functions etc.

Please check the change on line 1516, I added parentheses to avoid a warning. 
This changes the code behaviour, but to me it makes more sense now.

Original comment by raimo.ja...@gmail.com on 14 Jan 2011 at 12:06

Attachments:

GoogleCodeExporter commented 9 years ago
I just added a directx abstraction layer to d3dvideosink and I wouldn't mind if 
you could test it out w/ gcc. There's several more files to compile under 
gst-plugins-bad/sys/d3dvideosink/directx/ -- it should be obvious which ones. 
If you have the Makefile.am changes, it'd be nice to have them for when it's 
committed upstream.

Original comment by david.g.hoyt on 20 Jan 2011 at 12:33