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 709174 - Add avfvideosrc to the OS X build in applemedia
Add avfvideosrc to the OS X build in applemedia
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Mac OS
: Normal enhancement
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 711211
 
 
Reported: 2013-10-01 11:01 UTC by Andoni Morales
Modified: 2013-11-07 14:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH 1/8] applemedia: port avfvideosrc to 1.0 (7.84 KB, patch)
2013-10-30 11:32 UTC, Matthieu Bouron
none Details | Review
[PATCH 2/8] configure.ac: detect presence of AVFoundation (1.08 KB, patch)
2013-10-30 11:32 UTC, Matthieu Bouron
none Details | Review
[PATCH 3/8] applemedia: enable avfvideosrc if AVFoundation is present (3.74 KB, patch)
2013-10-30 11:33 UTC, Matthieu Bouron
none Details | Review
[PATCH 4/8] avfvideosrc: silence deprecated warning on osx >= 10.9 (874 bytes, patch)
2013-10-30 11:33 UTC, Matthieu Bouron
none Details | Review
[PATCH 5/8] avfvideosrc: remove trailing space (748 bytes, patch)
2013-10-30 11:34 UTC, Matthieu Bouron
none Details | Review
[PATCH 6/8] applemedia: use a dedicated queue for AVFoundation on OS (1.49 KB, patch)
2013-10-30 11:35 UTC, Matthieu Bouron
none Details | Review
[PATCH 7/8] avfvideosrc: check if low preset is available (1.01 KB, patch)
2013-10-30 11:35 UTC, Matthieu Bouron
none Details | Review
[PATCH 8/8] avfvideosrc: update caps if frame size has changed (3.57 KB, patch)
2013-10-30 11:35 UTC, Matthieu Bouron
none Details | Review
[PATCH 1/9] applemedia: port avfvideosrc to 1.0 (7.84 KB, patch)
2013-10-30 15:13 UTC, Matthieu Bouron
none Details | Review
[PATCH 2/8] configure.ac: detect presence of AVFoundation (1.08 KB, patch)
2013-10-30 15:14 UTC, Matthieu Bouron
committed Details | Review
[PATCH 3/9] avfvideosrc: only enable 1920x1080 preset on iOS (1.99 KB, patch)
2013-10-30 15:15 UTC, Matthieu Bouron
committed Details | Review
[PATCH 4/9] applemedia: enable avfvideosrc if AVFoundation is present (2.09 KB, patch)
2013-10-30 15:16 UTC, Matthieu Bouron
committed Details | Review
[PATCH 5/9] avfvideosrc: dispatch AVFoundation calls synchronously (3.18 KB, patch)
2013-10-30 15:16 UTC, Matthieu Bouron
committed Details | Review
[PATCH 6/9] avfvideosrc: remove trailing space (748 bytes, patch)
2013-10-30 15:17 UTC, Matthieu Bouron
committed Details | Review
[PATCH 7/9] avfvideosrc: use a dedicated queue for AVFoundation on OSX (1.50 KB, patch)
2013-10-30 15:18 UTC, Matthieu Bouron
none Details | Review
[PATCH 8/9] avfvideosrc: check if low preset is available (1.01 KB, patch)
2013-10-30 15:19 UTC, Matthieu Bouron
committed Details | Review
[PATCH 9/9] avfvideosrc: update caps if frame size has changed (3.45 KB, patch)
2013-10-30 15:19 UTC, Matthieu Bouron
none Details | Review
[PATCH 7/9] avfvideosrc: use a dedicated queue for AVFoundation calls (1.40 KB, patch)
2013-10-30 16:17 UTC, Matthieu Bouron
committed Details | Review
[PATCH 1/9] applemedia: port avfvideosrc to 1.0 (7.85 KB, patch)
2013-11-04 13:40 UTC, Matthieu Bouron
committed Details | Review
[PATCH 9/9] avfvideosrc: update caps if frame size has changed (3.42 KB, patch)
2013-11-04 13:42 UTC, Matthieu Bouron
committed Details | Review

Description Andoni Morales 2013-10-01 11:01:51 UTC
Right now we only build the qtkitvidesrc element, but the avfvideosrc element should also work in OS X and we should build it too although it's only supported in OS X 10.7
Comment 1 Matthieu Bouron 2013-10-29 10:13:23 UTC
Hello,

I started porting the avfvideosrc element to 1.0. It will also be enabled by the build system if it detects the presence of the AVFoundation framework.

Work in progress could be found here: http://cgit.collabora.com/git/user/mateo/gst-plugins-bad.git/log/?h=avfoundation_port_to_1.0
Comment 2 Matthieu Bouron 2013-10-30 11:32:07 UTC
Created attachment 258548 [details] [review]
[PATCH 1/8] applemedia: port avfvideosrc to 1.0
Comment 3 Matthieu Bouron 2013-10-30 11:32:45 UTC
Created attachment 258549 [details] [review]
[PATCH 2/8] configure.ac: detect presence of AVFoundation
Comment 4 Matthieu Bouron 2013-10-30 11:33:20 UTC
Created attachment 258550 [details] [review]
[PATCH 3/8] applemedia: enable avfvideosrc if AVFoundation is present
Comment 5 Matthieu Bouron 2013-10-30 11:33:48 UTC
Created attachment 258552 [details] [review]
[PATCH 4/8] avfvideosrc: silence deprecated warning on osx >= 10.9
Comment 6 Matthieu Bouron 2013-10-30 11:34:13 UTC
Created attachment 258553 [details] [review]
[PATCH 5/8] avfvideosrc: remove trailing space
Comment 7 Matthieu Bouron 2013-10-30 11:35:03 UTC
Created attachment 258554 [details] [review]
[PATCH 6/8] applemedia: use a dedicated queue for AVFoundation on OS
Comment 8 Matthieu Bouron 2013-10-30 11:35:30 UTC
Created attachment 258555 [details] [review]
[PATCH 7/8] avfvideosrc: check if low preset is available
Comment 9 Matthieu Bouron 2013-10-30 11:35:56 UTC
Created attachment 258556 [details] [review]
[PATCH 8/8] avfvideosrc: update caps if frame size has changed
Comment 10 Andoni Morales 2013-10-30 12:05:07 UTC
Review of attachment 258550 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +74,2 @@
 );
 

I think these changes should go in a different commit, they are not related to using AVFoundation :)

And for this last part I would do something like:
VIDEO_CAPS_BGRA (1280, 720))
#ifdef HAVE_IOS
 ";" VIDEO_CAPS_BGRA (1920, 1280))
#endif

I think it's easier to read and easier to know which resolutions are being added
Comment 11 Andoni Morales 2013-10-30 12:06:30 UTC
Review of attachment 258552 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +607,3 @@
   if (dispatchQueue != dispatch_get_current_queue())
       dispatch_sync (dispatchQueue, ^{});
+#pragma clang diagnostic pop

If this API is deprecated, we should use a replacement instead of silencing it
Comment 12 Andoni Morales 2013-10-30 12:06:32 UTC
Review of attachment 258552 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +607,3 @@
   if (dispatchQueue != dispatch_get_current_queue())
       dispatch_sync (dispatchQueue, ^{});
+#pragma clang diagnostic pop

If this API is deprecated, we should use a replacement instead of silencing it
Comment 13 Andoni Morales 2013-10-30 12:06:34 UTC
Review of attachment 258552 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +607,3 @@
   if (dispatchQueue != dispatch_get_current_queue())
       dispatch_sync (dispatchQueue, ^{});
+#pragma clang diagnostic pop

If this API is deprecated, we should use a replacement instead of silencing it
Comment 14 Andoni Morales 2013-10-30 12:06:41 UTC
Review of attachment 258552 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +607,3 @@
   if (dispatchQueue != dispatch_get_current_queue())
       dispatch_sync (dispatchQueue, ^{});
+#pragma clang diagnostic pop

If this API is deprecated, we should use a replacement instead of silencing it
Comment 15 Andoni Morales 2013-10-30 12:06:41 UTC
Review of attachment 258552 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +607,3 @@
   if (dispatchQueue != dispatch_get_current_queue())
       dispatch_sync (dispatchQueue, ^{});
+#pragma clang diagnostic pop

If this API is deprecated, we should use a replacement instead of silencing it
Comment 16 Andoni Morales 2013-10-30 12:06:41 UTC
Review of attachment 258552 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +607,3 @@
   if (dispatchQueue != dispatch_get_current_queue())
       dispatch_sync (dispatchQueue, ^{});
+#pragma clang diagnostic pop

If this API is deprecated, we should use a replacement instead of silencing it
Comment 17 Matthieu Bouron 2013-10-30 12:43:53 UTC
(In reply to comment #10)
> Review of attachment 258550 [details] [review]:
> 
> ::: sys/applemedia/avfvideosrc.m
> @@ +74,2 @@
>  );
> 
> 
> I think these changes should go in a different commit, they are not related to
> using AVFoundation :)
> 
> And for this last part I would do something like:
> VIDEO_CAPS_BGRA (1280, 720))
> #ifdef HAVE_IOS
>  ";" VIDEO_CAPS_BGRA (1920, 1280))
> #endif
> 
> I think it's easier to read and easier to know which resolutions are being
> added

Updated locally + patch splitted into:
avfvideosrc: only enable 1920x1080 preset on iOS
applemedia: enable avfvideosrc if AVFoundation is present
Comment 18 Matthieu Bouron 2013-10-30 12:59:18 UTC
(In reply to comment #13)
> Review of attachment 258552 [details] [review]:
> 
> ::: sys/applemedia/avfvideosrc.m
> @@ +607,3 @@
>    if (dispatchQueue != dispatch_get_current_queue())
>        dispatch_sync (dispatchQueue, ^{});
> +#pragma clang diagnostic pop
> 
> If this API is deprecated, we should use a replacement instead of silencing it

There is actually no replacement for this. I need to double check with the apple documentation and figure out it if it's actually neeeded.
Maybe you know more than me on the subject ?
Comment 19 Andoni Morales 2013-10-30 13:35:00 UTC
I think we can rework a bit how the dispatch queues are being used. We want to do all the AVFoundation work in the main queue, but we are using the following model:
  dispatch_async
  wait_for_queue_to_drain

instead of:
  dispatch_sync

Since it's a serial queue, wait_for_queue can block if its called from the same queue. get_current_queue was being used to avoid this situation.

But I think we can use dispatch_sync (mainQueue, for all the work in the main queue, because we actually want that to be executed synchronously and wait for that block executed in the main queue to be finished.

There is only one place where we use [self waitForWorkerQueueToDrain], in stop() and that can be safely replaced with  dispatch_sync (workerQueue, ^{}) because stop is never being called from the worker queue.
Comment 20 Matthieu Bouron 2013-10-30 15:13:36 UTC
Created attachment 258581 [details] [review]
[PATCH 1/9] applemedia: port avfvideosrc to 1.0
Comment 21 Matthieu Bouron 2013-10-30 15:14:08 UTC
Created attachment 258582 [details] [review]
[PATCH 2/8] configure.ac: detect presence of AVFoundation
Comment 22 Matthieu Bouron 2013-10-30 15:15:20 UTC
Created attachment 258583 [details] [review]
[PATCH 3/9] avfvideosrc: only enable 1920x1080 preset on iOS
Comment 23 Matthieu Bouron 2013-10-30 15:16:04 UTC
Created attachment 258584 [details] [review]
[PATCH 4/9] applemedia: enable avfvideosrc if AVFoundation is present
Comment 24 Matthieu Bouron 2013-10-30 15:16:54 UTC
Created attachment 258585 [details] [review]
[PATCH 5/9] avfvideosrc: dispatch AVFoundation calls synchronously
Comment 25 Matthieu Bouron 2013-10-30 15:17:40 UTC
Created attachment 258586 [details] [review]
[PATCH 6/9] avfvideosrc: remove trailing space
Comment 26 Matthieu Bouron 2013-10-30 15:18:25 UTC
Created attachment 258587 [details] [review]
[PATCH 7/9] avfvideosrc: use a dedicated queue for AVFoundation on OSX
Comment 27 Matthieu Bouron 2013-10-30 15:19:10 UTC
Created attachment 258589 [details] [review]
[PATCH 8/9] avfvideosrc: check if low preset is available
Comment 28 Matthieu Bouron 2013-10-30 15:19:49 UTC
Created attachment 258590 [details] [review]
[PATCH 9/9] avfvideosrc: update caps if frame size has changed
Comment 29 Andoni Morales 2013-10-30 15:42:30 UTC
Review of attachment 258587 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +160,2 @@
     workerQueue =
+        dispatch_queue_create ("org.freedesktop.gstreamer.avfvideosrc.output", NULL);

I believe that's not an OS X/iOS specific problem, it's just that in iOS there is always a main queue runnign and using gst-launch this queue might not exists. I would just create the queue if it doesn't exists (assuming dispatch_get_main_queue returns a NULL pointer in this scenario)
Comment 30 Andoni Morales 2013-10-30 15:42:32 UTC
Review of attachment 258587 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +160,2 @@
     workerQueue =
+        dispatch_queue_create ("org.freedesktop.gstreamer.avfvideosrc.output", NULL);

I believe that's not an OS X/iOS specific problem, it's just that in iOS there is always a main queue runnign and using gst-launch this queue might not exists. I would just create the queue if it doesn't exists (assuming dispatch_get_main_queue returns a NULL pointer in this scenario)
Comment 31 Matthieu Bouron 2013-10-30 16:17:54 UTC
Created attachment 258595 [details] [review]
[PATCH 7/9] avfvideosrc: use a dedicated queue for AVFoundation calls
Comment 32 Matthieu Bouron 2013-10-31 16:56:00 UTC
(In reply to comment #30)
> Review of attachment 258587 [details] [review]:
> 
> ::: sys/applemedia/avfvideosrc.m
> @@ +160,2 @@
>      workerQueue =
> +        dispatch_queue_create ("org.freedesktop.gstreamer.avfvideosrc.output",
> NULL);
> 
> I believe that's not an OS X/iOS specific problem, it's just that in iOS there
> is always a main queue runnign and using gst-launch this queue might not
> exists. I would just create the queue if it doesn't exists (assuming
> dispatch_get_main_queue returns a NULL pointer in this scenario)

For the record, dispatch_get_main_queue always returns the main queue since it is automatically created.
Comment 33 Matthieu Bouron 2013-11-04 13:40:54 UTC
Created attachment 258923 [details] [review]
[PATCH 1/9] applemedia: port avfvideosrc to 1.0

Minor cosmetic update.
Comment 34 Matthieu Bouron 2013-11-04 13:42:58 UTC
Created attachment 258924 [details] [review]
[PATCH 9/9] avfvideosrc: update caps if frame size has changed

Solve conflict with previous cosmetic update.
Comment 35 Andoni Morales 2013-11-07 14:31:30 UTC
commit 35587efdc9c0c803cc2b299873f9eb63bd2fc4fa
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Tue Oct 29 15:37:16 2013 +0000

    avfvideosrc: update caps if frame size has changed
    
    On OSX, setting the pixel format on the output reset the capture device
    to its native resolution, so we need to update the caps if the output
    frame size has changed before a proper solution is found.

commit fae79751ad864b64a0be5de5c70d407e5985af97
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Tue Oct 29 15:36:06 2013 +0000

    avfvideosrc: check if low preset is available

commit 7f807270fadbafe9fb69b71de1ca3823529c56dd
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Mon Oct 28 18:22:13 2013 +0000

    avfvideosrc: use a dedicated queue for AVFoundation calls
    
    Replace the main queue with a dedicated queue for AVFoundation calls
    so the execution on this queue won't block if the main queue
    is not running.

commit 5d612768a42d9bd862cfb0522017ea7fe9fd937c
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Tue Oct 29 17:33:11 2013 +0000

    avfvideosrc: remove trailing space

commit 19844fab4701a451ce06ddedfc5b0707ef920bd6
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Wed Oct 30 14:51:50 2013 +0000

    avfvideosrc: dispatch AVFoundation calls synchronously in the main queue

commit 0d74dc802acb8be3d28e9c1a844e09d9801e0a16
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Wed Oct 30 12:40:01 2013 +0000

    applemedia: enable avfvideosrc if AVFoundation is present

commit b6925d5c56aae3a1b40e8f99fe9cd8743815f8de
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Wed Oct 30 12:39:24 2013 +0000

    avfvideosrc: only enable 1920x1080 preset on iOS

commit f14cdcab58647dfa2b47aa0d314f16db2d91b740
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Mon Oct 28 11:20:27 2013 +0000

    configure.ac: detect presence of AVFoundation

commit 06d59e7829fca360dcbbb140ce9cb267f206b513
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Mon Oct 28 11:53:26 2013 +0000

    applemedia: port avfvideosrc to 1.0