diff mbox series

[FFmpeg-devel,1/4] Use appropriate method for device discovery, fix crash with bogus device index.

Message ID 20220130173045.32690-2-toots@rastageeks.org
State New
Headers show
Series Cleanup avfoundation input | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/commit_msg_ppc warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Romain Beauxis Jan. 30, 2022, 5:30 p.m. UTC 
From: Romain Beauxis <toots@rastageeks.org>

This updates the code for avfoundation to use modern device lookup APIs and also adds a check to avoid querying the video devices array beyound its maximum size.

---
 libavdevice/avfoundation.m | 71 ++++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 15 deletions(-)

Comments

Thilo Borgmann Jan. 31, 2022, 10:10 a.m. UTC | #1 
Hi,

Am 30.01.22 um 18:30 schrieb toots@rastageeks.org:
> From: Romain Beauxis <toots@rastageeks.org>
> 
> This updates the code for avfoundation to use modern device lookup APIs and also adds a check to avoid querying the video devices array beyound its maximum size.
> 
> ---
>   libavdevice/avfoundation.m | 71 ++++++++++++++++++++++++++++++--------
>   1 file changed, 56 insertions(+), 15 deletions(-)
> 
> diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
> index 0cd6e646d5..d8bcd98f81 100644
> --- a/libavdevice/avfoundation.m
> +++ b/libavdevice/avfoundation.m
> @@ -27,6 +27,7 @@
>   
>   #import <AVFoundation/AVFoundation.h>
>   #include <pthread.h>
> +#include <Availability.h>
>   
>   #include "libavutil/channel_layout.h"
>   #include "libavutil/pixdesc.h"
> @@ -764,8 +765,34 @@ static int avf_read_header(AVFormatContext *s)
[...]

the patch doesn't appear to be broken any more, though it does not apply to current HEAD

Thilos-Mac-mini:FFmpeg borgmann$ git apply ../patches/avfoundation/\[PATCH\ 1_4\]\ Use\ appropriate\ method\ for\ device\ discovery\,\ fix\ crash\ with\ bogus\ device\ index.\ -\ toots\@rastageeks.org\ -\ 2022-01-30\ 1830.eml
error: patch failed: libavdevice/avfoundation.m:27
error: libavdevice/avfoundation.m: patch does not apply

I don't see why it does not apply as the part around line 27 looks sane. Did you try to apply to HEAD?

-Thilo
Romain Beauxis Jan. 31, 2022, 2:19 p.m. UTC | #2 
Le lun. 31 janv. 2022 à 04:11, Thilo Borgmann <thilo.borgmann@mail.de> a
écrit :
>
> Hi,
>
> Am 30.01.22 um 18:30 schrieb toots@rastageeks.org:
> > From: Romain Beauxis <toots@rastageeks.org>
> >
> > This updates the code for avfoundation to use modern device lookup APIs
and also adds a check to avoid querying the video devices array beyound its
maximum size.
> >
> > ---
> >   libavdevice/avfoundation.m | 71 ++++++++++++++++++++++++++++++--------
> >   1 file changed, 56 insertions(+), 15 deletions(-)
> >
> > diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
> > index 0cd6e646d5..d8bcd98f81 100644
> > --- a/libavdevice/avfoundation.m
> > +++ b/libavdevice/avfoundation.m
> > @@ -27,6 +27,7 @@
> >
> >   #import <AVFoundation/AVFoundation.h>
> >   #include <pthread.h>
> > +#include <Availability.h>
> >
> >   #include "libavutil/channel_layout.h"
> >   #include "libavutil/pixdesc.h"
> > @@ -764,8 +765,34 @@ static int avf_read_header(AVFormatContext *s)
> [...]
>
> the patch doesn't appear to be broken any more, though it does not apply
to current HEAD
>
> Thilos-Mac-mini:FFmpeg borgmann$ git apply
../patches/avfoundation/\[PATCH\ 1_4\]\ Use\ appropriate\ method\ for\
device\ discovery\,\ fix\ crash\ with\ bogus\ device\ index.\ -\ toots\@
rastageeks.org\ -\ 2022-01-30\ 1830.eml
> error: patch failed: libavdevice/avfoundation.m:27
> error: libavdevice/avfoundation.m: patch does not apply
>
> I don't see why it does not apply as the part around line 27 looks sane.
Did you try to apply to HEAD?

It does seem to apply here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220130173045.32690-2-toots@rastageeks.org/
diff mbox series

Patch

diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 0cd6e646d5..d8bcd98f81 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -27,6 +27,7 @@ 
 
 #import <AVFoundation/AVFoundation.h>
 #include <pthread.h>
+#include <Availability.h>
 
 #include "libavutil/channel_layout.h"
 #include "libavutil/pixdesc.h"
@@ -764,8 +765,34 @@  static int avf_read_header(AVFormatContext *s)
     AVCaptureDevice *video_device = nil;
     AVCaptureDevice *audio_device = nil;
     // Find capture device
-    NSArray *devices = [AVCaptureDevice devicesWithMediaType:AVMediaTypeVideo];
-    NSArray *devices_muxed = [AVCaptureDevice devicesWithMediaType:AVMediaTypeMuxed];
+#if defined(__MAC_10_15) || (TARGET_OS_IPHONE && defined(__IPHONE_10_0))
+       AVCaptureDeviceDiscoverySession *discoverySession =
+            [AVCaptureDeviceDiscoverySession discoverySessionWithDeviceTypes:@[
+#if TARGET_OS_IPHONE
+                                                 AVCaptureDeviceTypeBuiltInDualCamera,
+                                                 AVCaptureDeviceTypeBuiltInDualWideCamera,
+                                                 AVCaptureDeviceTypeBuiltInUltraWideCamera,
+                                                 AVCaptureDeviceTypeBuiltInTrueDepthCamera,
+                                                 AVCaptureDeviceTypeBuiltInTelephotoCamera,
+#endif
+                                                 AVCaptureDeviceTypeBuiltInWideAngleCamera,
+                                                 AVCaptureDeviceTypeExternalUnknown
+                                             ]
+                                             mediaType:NULL
+                                             position:AVCaptureDevicePositionUnspecified];
+
+       NSMutableArray *devices       = [NSMutableArray array];
+       NSMutableArray *devices_muxed = [NSMutableArray array];
+       for (AVCaptureDevice *device in [discoverySession devices]) {
+           if ([device hasMediaType:AVMediaTypeVideo])
+               [devices addObject:device];
+           else if ([device hasMediaType:AVMediaTypeMuxed])
+               [devices_muxed addObject:device];
+       }
+#else
+       NSArray *devices = [AVCaptureDevice devicesWithMediaType:AVMediaTypeVideo];
+       NSArray *devices_muxed = [AVCaptureDevice devicesWithMediaType:AVMediaTypeMuxed];
+#endif
 
     ctx->num_video_devices = [devices count] + [devices_muxed count];
 
@@ -775,6 +802,21 @@  static int avf_read_header(AVFormatContext *s)
     CGGetActiveDisplayList(0, NULL, &num_screens);
 #endif
 
+    NSArray *audio_devices;
+#if defined(__MAC_10_15) || (TARGET_OS_IPHONE && defined(__IPHONE_10_0))
+    discoverySession =
+        [AVCaptureDeviceDiscoverySession discoverySessionWithDeviceTypes:@[
+                                             AVCaptureDeviceTypeBuiltInMicrophone,
+                                             AVCaptureDeviceTypeExternalUnknown
+                                         ]
+                                         mediaType:AVMediaTypeAudio
+                                         position:AVCaptureDevicePositionUnspecified];
+
+    audio_devices = [discoverySession devices];
+#else
+    audio_devices = [AVCaptureDevice devicesWithMediaType:AVMediaTypeAudio];
+#endif
+
     // List devices if requested
     if (ctx->list_devices) {
         int index = 0;
@@ -800,8 +842,7 @@  static int avf_read_header(AVFormatContext *s)
 #endif
 
         av_log(ctx, AV_LOG_INFO, "AVFoundation audio devices:\n");
-        devices = [AVCaptureDevice devicesWithMediaType:AVMediaTypeAudio];
-        for (AVCaptureDevice *device in devices) {
+        for (AVCaptureDevice *device in audio_devices) {
             const char *name = [[device localizedName] UTF8String];
             int index  = [devices indexOfObject:device];
             av_log(ctx, AV_LOG_INFO, "[%d] %s\n", index, name);
@@ -824,9 +865,12 @@  static int avf_read_header(AVFormatContext *s)
         if (ctx->video_device_index < ctx->num_video_devices) {
             if (ctx->video_device_index < [devices count]) {
                 video_device = [devices objectAtIndex:ctx->video_device_index];
-            } else {
+            } else if (ctx->video_device_index - [devices count] < [devices_muxed count]) {
                 video_device = [devices_muxed objectAtIndex:(ctx->video_device_index - [devices count])];
                 ctx->video_is_muxed = 1;
+            } else {
+                av_log(ctx, AV_LOG_ERROR, "Invalid video device index\n");
+                goto fail;
             }
         } else if (ctx->video_device_index < ctx->num_video_devices + num_screens) {
 #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
@@ -922,9 +966,7 @@  static int avf_read_header(AVFormatContext *s)
 
     // get audio device
     if (ctx->audio_device_index >= 0) {
-        NSArray *devices = [AVCaptureDevice devicesWithMediaType:AVMediaTypeAudio];
-
-        if (ctx->audio_device_index >= [devices count]) {
+        if (ctx->audio_device_index >= [audio_devices count]) {
             av_log(ctx, AV_LOG_ERROR, "Invalid audio device index\n");
             goto fail;
         }
@@ -935,15 +977,14 @@  static int avf_read_header(AVFormatContext *s)
         if (!strncmp(ctx->audio_filename, "default", 7)) {
             audio_device = [AVCaptureDevice defaultDeviceWithMediaType:AVMediaTypeAudio];
         } else {
-        NSArray *devices = [AVCaptureDevice devicesWithMediaType:AVMediaTypeAudio];
-
-        for (AVCaptureDevice *device in devices) {
-            if (!strncmp(ctx->audio_filename, [[device localizedName] UTF8String], strlen(ctx->audio_filename))) {
-                audio_device = device;
-                break;
+            for (AVCaptureDevice *device in audio_devices) {
+                const char *name = [[device localizedName] UTF8String];
+                if (!strncmp(ctx->audio_filename, name, strlen(ctx->audio_filename))) {
+                    audio_device = device;
+                    break;
+                }
             }
         }
-        }
 
         if (!audio_device) {
             av_log(ctx, AV_LOG_ERROR, "Audio device not found\n");