[Bug 704340] osvideosink: code refactoring: splitting obj-c code to separate files

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=704340
  GStreamer | gst-plugins-good | git

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ylatuya at gmail.com

--- Comment #17 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-07-17 10:33:03 UTC ---
(In reply to comment #12)
> (In reply to comment #7)
> Thanks, Sebastian.
> 
> 1) It is guaranteed, that osxwindow exists in case set_window_handle() was
> called in states higher than READY. Is it generally expected to be called in
> any state? If so, that should be changed. Didn't take it into account, sorry.

Yes

> 2) I think, I don't get it. Do you mean, we should also check for superview
> here? Or should it be moved to previous branch?

No I mean that this code is using ->osxwindow unconditionally, while a few
lines above you check if it's not NULL. So could crash probably.

(In reply to comment #15)
> (In reply to comment #10)
> > (From update of attachment 249302 [details] [details])
> > Not really reviewing this one here, would be great to split it into multiple
> > patches. One just moving the code around (and thus not needing a real review)
> > and others for the functional changes.
> 
> Yes, it's quite big now. The problem is that the splitted part was tightly
> connected to main part, so I'm afraid, there can't be a simple patch which just
> moves it to separate files. It's possible to reach, but *.m files would use
> parts of one another (with 'extern').
> 
> Sebastian, could you please suggest how can I split a commit in git, given that
> all these patches is committed in my local git repo? Never did this kind of
> things so it looks quite complex task for me.

Ok, leave it as is then :) It looks good, but I might've missed something. So
would prefer to have Andoni look at it too.

(In reply to comment #16)
> (In reply to comment #11)
> > Thanks for all the work on this, but independent of that please note that
> > currently it seems like a better option to switch to the sink in gst-plugins-gl
> > for 1.3/1.4 (to keep all the GL related code in a single place and allow better
> > GL integration). For 1.2 at least osxvideosink will still be used though.
> 
> Thanks for information, Sebastian, didn't know it. So, is this right, that
> gst-plugins-gl will be back and will work on all the GStreamer platforms?

That's the plan, and it should already work on X11, Wayland, OSX, Windows and
Android right now. X11, Wayland and Windows will use other, more optimal sinks
by default though (there's better API then GL available on these platforms).

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.