[FFmpeg-devel] [PATCH 1/4] libavdevice/avfoundation: add framerate and video size options

Matthieu Bouron matthieu.bouron at gmail.com
Fri Mar 27 14:34:38 CET 2015


On Sat, Mar 21, 2015 at 4:51 PM, Thilo Borgmann <thilo.borgmann at mail.de>
wrote:
[...]

> +/*
> + * Configure the video device using a run-time approach to access
> properties
> + * since formats, activeFormat are available since  iOS >= 7.0 or OSX >=
> 10.7
> + * and activeVideoMaxFrameDuration is available since i0S >= 7.0 and OSX
> >= 10.9
> + *
> + * The NSUndefinedKeyException must be handled by the caller of this
> function.
> + *
> + */
>
> Please make it a short doxygen comment: /**
>

Updated in attached patch.


>
> +static int configure_video_device(AVFormatContext *s, AVCaptureDevice
> *video_device)
> +{
> +    AVFContext *ctx = (AVFContext*)s->priv_data;
> +
> +    double framerate = av_q2d(ctx->framerate);
>
> +    NSObject *format = nil, *range = nil;
> +    NSObject *selected_format = nil, *selected_range = nil;
>
> Nit: Please split these in 4 lines.
>

Updated in attached patch.


>
>
> +    for (format in [video_device valueForKey:@"formats"]) {
> +        CMFormatDescriptionRef formatDescription;
> +        CMVideoDimensions dimensions;
> +
> +        formatDescription = (CMFormatDescriptionRef) [format
> performSelector:@selector(formatDescription)];
> +        dimensions =
> CMVideoFormatDescriptionGetDimensions(formatDescription);
> +
> +        if ((ctx->width == 0 && ctx->height == 0) ||
> +            (dimensions.width == ctx->width && dimensions.height ==
> ctx->height)) {
> +
> +            selected_format = format;
> +
> +            for (range in [format valueForKey:@"videoSupportedFrameRateRanges"])
> {
> +                double max_framerate;
> +
> +                [[range valueForKey:@"maxFrameRate"]
> getValue:&max_framerate];
> +                if (abs (framerate - max_framerate) < 0.00001) {
>
> +
>
> Unneeded blank line.
>

Updated in attached patch.


>
> +                    selected_range = range;
> +                    break;
> +                }
> +            }
> +        }
> +    }
> +
>
> +    if (!selected_format) {
> +        av_log(s, AV_LOG_ERROR, "Selected video size (%dx%d) is not
> supported
> by the device\n",
> +            ctx->width, ctx->height);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (!selected_range) {
> +        av_log(s, AV_LOG_ERROR, "Selected framerate (%f) is not supported
> by
> the device\n",
> +            framerate);
> +        return AVERROR(EINVAL);
> +    }
>
> In case of an unsupported format/rate, please print a list of supported
> formats,
> like the pixel format test does.
>

Updated in attached patch. I've also replaced the wrong usage of abs with
fabs.


>
>
> +
> +    if ([video_device lockForConfiguration:NULL] == YES) {
> +        NSValue *min_frame_duration = [selected_range
> valueForKey:@"minFrameDuration"];
> +
>
> +        [video_device setValue:selected_format forKey:@"activeFormat"];
> +        [video_device setValue:min_frame_duration
> forKey:@"activeVideoMinFrameDuration"];
> +        [video_device setValue:min_frame_duration
> forKey:@"activeVideoMaxFrameDuration"];
>                                 ^^^
> Should be max_frame_duration in the third line.
> IIRC there is functionality in the API to check if setting
> min/max_frame_duration is available for the device, isn't it? Please
> check, in
> case there it is not, there is an alternative way to set the framerate
> (IIRC).
>

It is purely intentionnal to use the min_frame_duration for the
activeVideoMaxFrameDuration to acheive a better consistant framerate.
There is no way to check if the range is accepted by the device but as
stated by the documentation, applied range values that does not come from
ranges returned by the API will throw an exception.
There is an alternative way to set the framerate using
AVCaptureConnection.setVideo(Min|Max)FrameDuration but it has been
deprecated in iOS 7 and I do not intend to support it.


>
>
> +    } else {
> +        av_log(s, AV_LOG_ERROR, "Could not lock device for
> configuration");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    return 0;
> +}
> +
>  static int add_video_device(AVFormatContext *s, AVCaptureDevice
> *video_device)
>  {
>      AVFContext *ctx = (AVFContext*)s->priv_data;
> +    int ret;
>      NSError *error  = nil;
>      AVCaptureInput* capture_input = nil;
>      struct AVFPixelFormatSpec pxl_fmt_spec;
> @@ -300,6 +372,18 @@ static int add_video_device(AVFormatContext *s,
> AVCaptureDevice *video_device)
>          return 1;
>      }
>
> +    // Configure device framerate and video size
> +    @try {
> +        if ((ret = configure_video_device(s, video_device)) < 0) {
> +            return ret;
> +        }
> +    } @catch (NSException *exception) {
> +        if (![[exception name] isEqualToString:NSUndefinedKeyException]) {
> +          av_log (s, AV_LOG_ERROR, "An error occured: %s",
> [exception.reason
> UTF8String]);
> +          return AVERROR_EXTERNAL;
> +        }
> +    }
> +
>      // select pixel format
>      pxl_fmt_spec.ff_id = AV_PIX_FMT_NONE;
>
> @@ -549,6 +633,7 @@ static int get_audio_config(AVFormatContext *s)
>  static int avf_read_header(AVFormatContext *s)
>  {
>      NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
> +    int capture_screen      = 0;
>      uint32_t num_screens    = 0;
>      AVFContext *ctx         = (AVFContext*)s->priv_data;
>      AVCaptureDevice *video_device = nil;
> @@ -616,7 +701,13 @@ static int avf_read_header(AVFormatContext *s)
>              CGDirectDisplayID screens[num_screens];
>              CGGetActiveDisplayList(num_screens, screens, &num_screens);
>              AVCaptureScreenInput* capture_screen_input =
> [[[AVCaptureScreenInput alloc]
> initWithDisplayID:screens[ctx->video_device_index
> - ctx->num_video_devices]] autorelease];
> +
> +            if (ctx->framerate.num > 0) {
> +                capture_screen_input.minFrameDuration =
> CMTimeMake(ctx->framerate.den, ctx->framerate.num);
> +            }
> +
>              video_device = (AVCaptureDevice*) capture_screen_input;
> +            capture_screen = 1;
>  #endif
>           } else {
>              av_log(ctx, AV_LOG_ERROR, "Invalid device index\n");
> @@ -645,6 +736,11 @@ static int avf_read_header(AVFormatContext *s)
>                  AVCaptureScreenInput* capture_screen_input =
> [[[AVCaptureScreenInput alloc] initWithDisplayID:screens[idx]]
> autorelease];
>                  video_device = (AVCaptureDevice*) capture_screen_input;
>                  ctx->video_device_index = ctx->num_video_devices + idx;
> +                capture_screen = 1;
> +
> +                if (ctx->framerate.num > 0) {
> +                    capture_screen_input.minFrameDuration =
> CMTimeMake(ctx->framerate.den, ctx->framerate.num);
> +                }
>              }
>          }
>  #endif
> @@ -715,6 +811,12 @@ static int avf_read_header(AVFormatContext *s)
>
>      [ctx->capture_session startRunning];
>
> +    /* Unlock device configuration only after the session is started so it
> +     * does not reset the capture formats */
> +    if (!capture_screen) {
> +        [video_device unlockForConfiguration];
> +    }
> +
>      if (video_device && get_video_config(s)) {
>          goto fail;
>      }
> @@ -853,6 +955,8 @@ static const AVOption options[] = {
>      { "video_device_index", "select video device by index for devices
> with same
> name (starts at 0)", offsetof(AVFContext, video_device_index),
> AV_OPT_TYPE_INT,
> {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_DECODING_PARAM },
>      { "audio_device_index", "select audio device by index for devices
> with same
> name (starts at 0)", offsetof(AVFContext, audio_device_index),
> AV_OPT_TYPE_INT,
> {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_DECODING_PARAM },
>      { "pixel_format", "set pixel format", offsetof(AVFContext,
> pixel_format),
> AV_OPT_TYPE_PIXEL_FMT, {.i64 = AV_PIX_FMT_YUV420P}, 0, INT_MAX,
> AV_OPT_FLAG_DECODING_PARAM},
> +    { "framerate", "set frame rate", offsetof(AVFContext, framerate),
> AV_OPT_TYPE_VIDEO_RATE, {.str = "ntsc"}, 0, 0, AV_OPT_FLAG_DECODING_PARAM
> },
>
> +    { "size", "set video size", offsetof(AVFContext, width),
> AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, AV_OPT_FLAG_DECODING_PARAM },
>
> Please change the option's name to "video_size" to be more consistent.
>

Updated in attached patch.

Thanks for the review,
Matthieu

[...]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libavdevice-avfoundation-add-framerate-and-video-siz.patch
Type: application/octet-stream
Size: 8782 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150327/926ee5fe/attachment.obj>


More information about the ffmpeg-devel mailing list