After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 572701 - [osxvideosink] improvements and GNUstep compatibility
[osxvideosink] improvements and GNUstep compatibility
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal enhancement
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-02-22 01:03 UTC by Julien Isorce
Modified: 2009-08-10 23:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
osxvideosink patch (37.55 KB, patch)
2009-02-22 01:05 UTC, Julien Isorce
rejected Details | Review

Description Julien Isorce 2009-02-22 01:03:51 UTC
Well, it's a patch for osxvideosink.

I developped it on win32 ... yes it's a little bit strange ... using GNUstep environnement (it includes a modified msys+mingw+cocoaHeaderAndLibs+win32backend).

Anyway. I am curious to know if the patch works (compile then run) on MacOS.

Main changes:
-use a different thread to run the Application main loop. This thread is created using glib and it's considered as the main thread as far as GNUstep/Cocoa is concerned.
(just like it's done in other video renders: ximagesink, xv, gl, directdraw)
-use a NSWindow. (the previous code was only using a NSView without a NSWindow, maybe it only works on MacOS)
-inits and view updates are made using "performOnMainThread". And so the OpenGL context is made current only one time, when initializing it.
-avoid a memcpy which was really useless.
-avoid the use of CGL.
-use glTexImage2D with NULL. (just the size is important)
-handle RGBx. (I have no Apple_422 extension on win32)
-implement the gstxoverlay interface.

And some other stuffs that I do not remember right now.

I put some #ifdef GNUSTEP, because of some specific things. 
So I am not sure my patch will compile the first time.
And I have no Mac computer.

Let me know if that works on MacOS.

Sincerely

Cap
Comment 1 Julien Isorce 2009-02-22 01:05:00 UTC
Created attachment 129233 [details] [review]
osxvideosink patch
Comment 2 Michael Smith 2009-02-23 18:51:07 UTC
I don't want to take code to reintroduce the NSApp/top-level window handling. It doesn't work in real osx applications, and we just shouldn't support this.

I'm also not going to review this patch while it has random re-indenting in it. If there's a need to reindent to make things consistent, we can do that either before or after this patch, but not in the same patch.

Things I would like to see from this:
 - avoiding unnecessary memcpy
 - implementing XOverlay
 - handling more colour formats 
 - I have no idea about what CGL is/why you want to avoid it. But if that's useful, sure.

So if you want to provide a patch like that, I'd be happy to review it, and test on osx.
Comment 3 Julien Isorce 2009-02-23 19:15:25 UTC
Hi,

Well I am not suprised of your answer.

"I don't want to take code to reintroduce the NSApp/top-level window handling."
But not-having-NSApp only works on MacOS. (not on GNUstep env ans other)
Moreover I think it's the only way to support gstxoverlay interface.

I do not want to spend time on splitting my patch because I think it's ok.

I would jut find someone (who has a MacOS machine) who can confirm or not that it works (or not) on a Mac computer.

I should not ask this on the bugtracker.

Sorry for the noise.

Julien
Comment 4 Michael Smith 2009-02-23 19:18:56 UTC
Ok, if you're not willing to split out the useful parts of this patch, then I'm just going to say 'no' to it.

I don't see any value in trying to build a patch that has no prospect of going upstream.
Comment 5 Julien Isorce 2009-02-23 20:33:54 UTC
Sure, I needed the info (compile or not, work or not) in order to start/finish the MacOS back for gst-plugings-gl. I first tried to compile and use the osxvideosink on a GNUstep environnement. But I finnaly did a lot of changes on it (note that only 5 lines of the patch are specific to GNUstep).
I thought it would be usefull to share the experience that's why I did a git diff . > plugin.patch)
But it seems I did too many changes (+bad indent) to review it easily.
But I can say that I saw a lot of strange things in this plugin and that's why also I rewrote a lot of parts.
Anyway, forgot this bug.

Again sorry for the noise.

I close the bug.

Sincerely

Julien