| Message ID | DA019F53-F665-4AEB-A9B9-F2004C376B71@rastageeks.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v8,1/3] libavdevice/avfoundation.m: use AudioConvert, extend supported formats | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configurex86 | warning | Failed to apply patch |
| andriy/configureppc | warning | Failed to apply patch |
On 31 Dec 2021, at 18:43, Romain Beauxis wrote: > * Use a CMSimpleQueueEnqueue with maximum length to queue and process > incoming audio and video frames. > * Log avfoundation errors. > * Use AVERROR_EXTERNAL instead of AVERROR(EIO) in avfoundation errors. > > Signed-off-by: Romain Beauxis <toots@rastageeks.org> > — > [Sorry for the noise but an issue came up with the previous set] > > This is the second patch of a series of 3 that fix, cleanup and > enhance the > avfoundation implementation for libavdevice. > > These patches come from an actual user-facing application relying on > libavdevice’s implementation of avfoundation audio input. Without > them, > Avfoundation is practically unusable as it will: > * Refuse to process certain specific audio input format that are > actually > returned by the OS for some users (packed PCM audio) > * Drop audio frames, resulting in corrupted audio input. This might > have been > unnoticed with video frames but this makes avfoundation essentially > unusable > for audio. > > The patches are now being included in our production build so they are > tested > and usable in production. > > Changelog for this patch: > * v2: None > * v3: None > * v4: None > * v5: Fix indentation/wrapping > * v6: None > * v7: Removed use of kAudioConverterPropertyCalculateOutputBufferSize > to calculate output buffer size. The calculation is trivial and this > call was > randomly failing for no reason > * v8: Fix memory leak when video or audio queue is full > > libavdevice/avfoundation.m | 194 +++++++++++++++++++------------------ > 1 file changed, 100 insertions(+), 94 deletions(-) > > diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m > index 738cd93375..36f9fdc53d 100644 > --- a/libavdevice/avfoundation.m > +++ b/libavdevice/avfoundation.m > @@ -26,7 +26,7 @@ > */ > > #import <AVFoundation/AVFoundation.h> > -#include <pthread.h> > +#import <CoreMedia/CoreMedia.h> > > #include "libavutil/channel_layout.h" > #include "libavutil/pixdesc.h" > @@ -39,6 +39,11 @@ > #include "libavutil/imgutils.h" > #include "avdevice.h" > > +#define av_log_avfoundation_error(s, str, err) \ > + av_log(s, AV_LOG_ERROR, "Avfoundation: %s, %s\n", str, \ nitpick: should probably be AVFoundation, no? > + [[[NSError errorWithDomain:NSOSStatusErrorDomain code:err > userInfo:nil] localizedDescription] UTF8String] \ > + ) > + The errorWithDomain: returns an autorelease NSError, however there is no autorelease pool. Either make this a function with an @autorelease pool or use [[… alloc] init…] instead, and release the NSError. > static const int avf_time_base = 1000000; > > static const AVRational avf_time_base_q = { > @@ -80,13 +85,12 @@ > { AV_PIX_FMT_NONE, 0 } > }; > > +#define MAX_QUEUED_FRAMES 10 > + > typedef struct > { > AVClass* class; > > - int frames_captured; > - int audio_frames_captured; > - pthread_mutex_t frame_lock; > id avf_delegate; > id avf_audio_delegate; > > @@ -122,8 +126,8 @@ > AVCaptureSession *capture_session; > AVCaptureVideoDataOutput *video_output; > AVCaptureAudioDataOutput *audio_output; > - CMSampleBufferRef current_frame; > - CMSampleBufferRef current_audio_frame; > + CMSimpleQueueRef audio_frames_queue; > + CMSimpleQueueRef video_frames_queue; > > AVCaptureDevice *observed_device; > #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070 > @@ -132,16 +136,6 @@ > int observed_quit; > } AVFContext; > > -static void lock_frames(AVFContext* ctx) > -{ > - pthread_mutex_lock(&ctx->frame_lock); > -} > - > -static void unlock_frames(AVFContext* ctx) > -{ > - pthread_mutex_unlock(&ctx->frame_lock); > -} > - > /** FrameReciever class - delegate for AVCaptureSession > */ > @interface AVFFrameReceiver : NSObject > @@ -219,17 +213,13 @@ - (void) captureOutput:(AVCaptureOutput > *)captureOutput > didOutputSampleBuffer:(CMSampleBufferRef)videoFrame > fromConnection:(AVCaptureConnection *)connection > { > - lock_frames(_context); > + OSStatus ret = CMSimpleQueueEnqueue(_context->video_frames_queue, > videoFrame); > > - if (_context->current_frame != nil) { > - CFRelease(_context->current_frame); > + if (ret != noErr) { > + av_log_avfoundation_error(_context, "Error while queueing video > frame", ret); > } > > - _context->current_frame = > (CMSampleBufferRef)CFRetain(videoFrame); > - > - unlock_frames(_context); > - > - ++_context->frames_captured; > + CFRetain(videoFrame); > } > > @end > @@ -263,17 +253,13 @@ - (void) captureOutput:(AVCaptureOutput > *)captureOutput > didOutputSampleBuffer:(CMSampleBufferRef)audioFrame > fromConnection:(AVCaptureConnection *)connection > { > - lock_frames(_context); > + OSStatus ret = CMSimpleQueueEnqueue(_context->audio_frames_queue, > audioFrame); > > - if (_context->current_audio_frame != nil) { > - CFRelease(_context->current_audio_frame); > + if (ret != noErr) { > + av_log_avfoundation_error(_context, "Error while queueing audio > frame", ret); > } > > - _context->current_audio_frame = > (CMSampleBufferRef)CFRetain(audioFrame); > - > - unlock_frames(_context); > - > - ++_context->audio_frames_captured; > + CFRetain(audioFrame); > } > > @end > @@ -288,6 +274,30 @@ static void destroy_context(AVFContext* ctx) > [ctx->avf_delegate release]; > [ctx->avf_audio_delegate release]; > > + CMSampleBufferRef frame; > + > + if (ctx->video_frames_queue) { > + frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); > + while (frame) { > + CFRelease(frame); > + frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); > + } > + > + CFRelease(ctx->video_frames_queue); > + ctx->video_frames_queue = NULL; > + } > + > + if (ctx->audio_frames_queue) { > + frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); > + while (frame) { > + CFRelease(frame); > + frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); > + } > + > + CFRelease(ctx->audio_frames_queue); > + ctx->audio_frames_queue = NULL; > + } > + > ctx->capture_session = NULL; > ctx->video_output = NULL; > ctx->audio_output = NULL; > @@ -298,12 +308,6 @@ static void destroy_context(AVFContext* ctx) > AudioConverterDispose(ctx->audio_converter); > ctx->audio_converter = NULL; > } > - > - pthread_mutex_destroy(&ctx->frame_lock); > - > - if (ctx->current_frame) { > - CFRelease(ctx->current_frame); > - } > } > > static void parse_device_name(AVFormatContext *s) > @@ -631,18 +635,18 @@ static int get_video_config(AVFormatContext *s) > } > > // Take stream info from the first frame. > - while (ctx->frames_captured < 1) { > + while (CMSimpleQueueGetCount(ctx->video_frames_queue) < 1) { > CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.1, YES); > } > > - lock_frames(ctx); > + CMSampleBufferRef frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); > > ctx->video_stream_index = stream->index; > > avpriv_set_pts_info(stream, 64, 1, avf_time_base); > > - image_buffer = CMSampleBufferGetImageBuffer(ctx->current_frame); > - block_buffer = CMSampleBufferGetDataBuffer(ctx->current_frame); > + image_buffer = CMSampleBufferGetImageBuffer(frame); > + block_buffer = CMSampleBufferGetDataBuffer(frame); > > if (image_buffer) { > image_buffer_size = CVImageBufferGetEncodedSize(image_buffer); > @@ -658,10 +662,7 @@ static int get_video_config(AVFormatContext *s) > stream->codecpar->format = ctx->pixel_format; > } > > - CFRelease(ctx->current_frame); > - ctx->current_frame = nil; > - > - unlock_frames(ctx); > + CFRelease(frame); > > return 0; > } > @@ -681,27 +682,27 @@ static int get_audio_config(AVFormatContext *s) > } > > // Take stream info from the first frame. > - while (ctx->audio_frames_captured < 1) { > + while (CMSimpleQueueGetCount(ctx->audio_frames_queue) < 1) { > CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.1, YES); > } > > - lock_frames(ctx); > + CMSampleBufferRef frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); > > ctx->audio_stream_index = stream->index; > > avpriv_set_pts_info(stream, 64, 1, avf_time_base); > > - format_desc = > CMSampleBufferGetFormatDescription(ctx->current_audio_frame); > + format_desc = CMSampleBufferGetFormatDescription(frame); > const AudioStreamBasicDescription *input_format = > CMAudioFormatDescriptionGetStreamBasicDescription(format_desc); > > if (!input_format) { > - unlock_frames(ctx); > + CFRelease(frame); > av_log(s, AV_LOG_ERROR, "audio format not available\n"); > return 1; > } > > if (input_format->mFormatID != kAudioFormatLinearPCM) { > - unlock_frames(ctx); > + CFRelease(frame); > av_log(s, AV_LOG_ERROR, "only PCM audio format are supported > at the moment\n"); > return 1; > } > @@ -781,16 +782,13 @@ static int get_audio_config(AVFormatContext *s) > if (must_convert) { > OSStatus ret = AudioConverterNew(input_format, &output_format, > &ctx->audio_converter); > if (ret != noErr) { > - unlock_frames(ctx); > - av_log(s, AV_LOG_ERROR, "Error while allocating audio > converter\n"); > + CFRelease(frame); > + av_log_avfoundation_error(s, "error while creating audio > converter", ret); > return 1; > } > } > > - CFRelease(ctx->current_audio_frame); > - ctx->current_audio_frame = nil; > - > - unlock_frames(ctx); > + CFRelease(frame); > > return 0; > } > @@ -808,8 +806,6 @@ static int avf_read_header(AVFormatContext *s) > > ctx->num_video_devices = [devices count] + [devices_muxed count]; > > - pthread_mutex_init(&ctx->frame_lock, NULL); > - > #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070 > CGGetActiveDisplayList(0, NULL, &num_screens); > #endif > @@ -1010,6 +1006,21 @@ static int avf_read_header(AVFormatContext *s) > // Initialize capture session > ctx->capture_session = [[AVCaptureSession alloc] init]; > > + OSStatus ret; > + ret = CMSimpleQueueCreate(kCFAllocatorDefault, MAX_QUEUED_FRAMES, > &ctx->video_frames_queue); > + > + if (ret != noErr) { > + av_log_avfoundation_error(s, "error while creating frame > queue", ret); > + goto fail; > + } > + > + ret = CMSimpleQueueCreate(kCFAllocatorDefault, MAX_QUEUED_FRAMES, > &ctx->audio_frames_queue); > + > + if (ret != noErr) { > + av_log_avfoundation_error(s, "error while creating frame > queue", ret); > + goto fail; > + } > + > if (video_device && add_video_device(s, video_device)) { > goto fail; > } > @@ -1039,7 +1050,8 @@ static int avf_read_header(AVFormatContext *s) > fail: > [pool release]; > destroy_context(ctx); > - return AVERROR(EIO); > + av_log(s, AV_LOG_ERROR, "Error while opening AVfoundation capture > session\n"); > + return AVERROR_EXTERNAL; > } > > static int copy_cvpixelbuffer(AVFormatContext *s, > @@ -1088,38 +1100,35 @@ static int copy_cvpixelbuffer(AVFormatContext > *s, > static int avf_read_packet(AVFormatContext *s, AVPacket *pkt) > { > OSStatus ret; > + int status; > AVFContext* ctx = (AVFContext*)s->priv_data; > > do { > - CVImageBufferRef image_buffer; > - CMBlockBufferRef block_buffer; > - lock_frames(ctx); > - > - if (ctx->current_frame != nil) { > - int status; > + if (1 <= CMSimpleQueueGetCount(ctx->video_frames_queue)) { > int length = 0; > - > - image_buffer = > CMSampleBufferGetImageBuffer(ctx->current_frame); > - block_buffer = > CMSampleBufferGetDataBuffer(ctx->current_frame); > + CMSampleBufferRef video_frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); > + CVImageBufferRef image_buffer = > CMSampleBufferGetImageBuffer(video_frame);; > + CMBlockBufferRef block_buffer = > CMSampleBufferGetDataBuffer(video_frame); > > if (image_buffer != nil) { > length = (int)CVPixelBufferGetDataSize(image_buffer); > } else if (block_buffer != nil) { > length = > (int)CMBlockBufferGetDataLength(block_buffer); > } else { > - unlock_frames(ctx); > + CFRelease(video_frame); > return AVERROR(EINVAL); > } > > - if (av_new_packet(pkt, length) < 0) { > - unlock_frames(ctx); > - return AVERROR(EIO); > + status = av_new_packet(pkt, length); > + if (status < 0) { > + CFRelease(video_frame); > + return status; > } > > CMItemCount count; > CMSampleTimingInfo timing_info; > > - if > (CMSampleBufferGetOutputSampleTimingInfoArray(ctx->current_frame, 1, > &timing_info, &count) == noErr) { > + if > (CMSampleBufferGetOutputSampleTimingInfoArray(video_frame, 1, > &timing_info, &count) == noErr) { > AVRational timebase_q = av_make_q(1, > timing_info.presentationTimeStamp.timescale); > pkt->pts = pkt->dts = > av_rescale_q(timing_info.presentationTimeStamp.value, timebase_q, > avf_time_base_q); > } > @@ -1133,18 +1142,18 @@ static int avf_read_packet(AVFormatContext *s, > AVPacket *pkt) > status = 0; > ret = CMBlockBufferCopyDataBytes(block_buffer, 0, > pkt->size, pkt->data); > if (ret != kCMBlockBufferNoErr) { > - status = AVERROR(EIO); > + av_log_avfoundation_error(s, "error while copying > buffer data", ret); > + status = AVERROR_EXTERNAL; > } > } > - CFRelease(ctx->current_frame); > - ctx->current_frame = nil; > + CFRelease(video_frame); > > if (status < 0) { > - unlock_frames(ctx); > return status; > } > - } else if (ctx->current_audio_frame != nil) { > - CMBlockBufferRef block_buffer = > CMSampleBufferGetDataBuffer(ctx->current_audio_frame); > + } else if (1 <= > CMSimpleQueueGetCount(ctx->audio_frames_queue)) { > + CMSampleBufferRef audio_frame = > (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); > + CMBlockBufferRef block_buffer = > CMSampleBufferGetDataBuffer(audio_frame); > > size_t input_size = > CMBlockBufferGetDataLength(block_buffer); > int buffer_size = input_size / ctx->audio_buffers; > @@ -1170,8 +1179,9 @@ static int avf_read_packet(AVFormatContext *s, > AVPacket *pkt) > > if (ret != kCMBlockBufferNoErr) { > av_free(input_buffer); > - unlock_frames(ctx); > - return AVERROR(EIO); > + CFRelease(audio_frame); > + av_log_avfoundation_error(s, "error while > accessing audio buffer data", ret); > + return AVERROR_EXTERNAL; > } > } > > @@ -1188,23 +1198,25 @@ static int avf_read_packet(AVFormatContext *s, > AVPacket *pkt) > av_free(input_buffer); > > if (ret != noErr) { > - unlock_frames(ctx); > - return AVERROR(EIO); > + CFRelease(audio_frame); > + av_log_avfoundation_error(s, "error while > converting audio data", ret); > + return AVERROR_EXTERNAL; > } > > pkt->size = output_buffer.mBuffers[0].mDataByteSize; > } else { > ret = CMBlockBufferCopyDataBytes(block_buffer, 0, > pkt->size, pkt->data); > if (ret != kCMBlockBufferNoErr) { > - unlock_frames(ctx); > - return AVERROR(EIO); > + CFRelease(audio_frame); > + av_log_avfoundation_error(s, "error while > copying audio data", ret); > + return AVERROR_EXTERNAL; > } > } > > CMItemCount count; > CMSampleTimingInfo timing_info; > > - if > (CMSampleBufferGetOutputSampleTimingInfoArray(ctx->current_audio_frame, > 1, &timing_info, &count) == noErr) { > + if > (CMSampleBufferGetOutputSampleTimingInfoArray(audio_frame, 1, > &timing_info, &count) == noErr) { > AVRational timebase_q = av_make_q(1, > timing_info.presentationTimeStamp.timescale); > pkt->pts = pkt->dts = > av_rescale_q(timing_info.presentationTimeStamp.value, timebase_q, > avf_time_base_q); > } > @@ -1212,21 +1224,15 @@ static int avf_read_packet(AVFormatContext *s, > AVPacket *pkt) > pkt->stream_index = ctx->audio_stream_index; > pkt->flags |= AV_PKT_FLAG_KEY; > > - CFRelease(ctx->current_audio_frame); > - ctx->current_audio_frame = nil; > - > - unlock_frames(ctx); > + CFRelease(audio_frame); > } else { > pkt->data = NULL; > - unlock_frames(ctx); > if (ctx->observed_quit) { > return AVERROR_EOF; > } else { > return AVERROR(EAGAIN); > } > } > - > - unlock_frames(ctx); > } while (!pkt->data); > > return 0; > -- > 2.32.0 (Apple Git-132) > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Le mer. 5 janv. 2022 à 08:50, Marvin Scholz <epirat07@gmail.com> a écrit : > > On 31 Dec 2021, at 18:43, Romain Beauxis wrote: > > > * Use a CMSimpleQueueEnqueue with maximum length to queue and process > > incoming audio and video frames. > > * Log avfoundation errors. > > * Use AVERROR_EXTERNAL instead of AVERROR(EIO) in avfoundation errors. > > > > Signed-off-by: Romain Beauxis <toots@rastageeks.org> > > — > > [Sorry for the noise but an issue came up with the previous set] > > > > This is the second patch of a series of 3 that fix, cleanup and > > enhance the > > avfoundation implementation for libavdevice. > > > > These patches come from an actual user-facing application relying on > > libavdevice’s implementation of avfoundation audio input. Without > > them, > > Avfoundation is practically unusable as it will: > > * Refuse to process certain specific audio input format that are > > actually > > returned by the OS for some users (packed PCM audio) > > * Drop audio frames, resulting in corrupted audio input. This might > > have been > > unnoticed with video frames but this makes avfoundation essentially > > unusable > > for audio. > > > > The patches are now being included in our production build so they are > > tested > > and usable in production. > > > > Changelog for this patch: > > * v2: None > > * v3: None > > * v4: None > > * v5: Fix indentation/wrapping > > * v6: None > > * v7: Removed use of kAudioConverterPropertyCalculateOutputBufferSize > > to calculate output buffer size. The calculation is trivial and this > > call was > > randomly failing for no reason > > * v8: Fix memory leak when video or audio queue is full > > > > libavdevice/avfoundation.m | 194 +++++++++++++++++++------------------ > > 1 file changed, 100 insertions(+), 94 deletions(-) > > > > diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m > > index 738cd93375..36f9fdc53d 100644 > > --- a/libavdevice/avfoundation.m > > +++ b/libavdevice/avfoundation.m > > @@ -26,7 +26,7 @@ > > */ > > > > #import <AVFoundation/AVFoundation.h> > > -#include <pthread.h> > > +#import <CoreMedia/CoreMedia.h> > > > > #include "libavutil/channel_layout.h" > > #include "libavutil/pixdesc.h" > > @@ -39,6 +39,11 @@ > > #include "libavutil/imgutils.h" > > #include "avdevice.h" > > > > +#define av_log_avfoundation_error(s, str, err) \ > > + av_log(s, AV_LOG_ERROR, "Avfoundation: %s, %s\n", str, \ > > nitpick: should probably be AVFoundation, no? Done! > > + [[[NSError errorWithDomain:NSOSStatusErrorDomain code:err > > userInfo:nil] localizedDescription] UTF8String] \ > > + ) > > + > > The errorWithDomain: returns an autorelease NSError, however there is no > autorelease pool. > Either make this a function with an @autorelease pool or use [[… > alloc] init…] instead, and > release the NSError. That's right, thanks for pointing that out. Just sent a v9 version of the patchset fixing that and also dropping the log level for the error returned when the queue is full. In practice, the queue can become full pretty often, for instance when waiting on a remote output connection so these logs end up polluting the output in situations that are not really problematic. Thanks for looking into this! -- Romain
diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m index 738cd93375..36f9fdc53d 100644 --- a/libavdevice/avfoundation.m +++ b/libavdevice/avfoundation.m @@ -26,7 +26,7 @@ */ #import <AVFoundation/AVFoundation.h> -#include <pthread.h> +#import <CoreMedia/CoreMedia.h> #include "libavutil/channel_layout.h" #include "libavutil/pixdesc.h" @@ -39,6 +39,11 @@ #include "libavutil/imgutils.h" #include "avdevice.h" +#define av_log_avfoundation_error(s, str, err) \ + av_log(s, AV_LOG_ERROR, "Avfoundation: %s, %s\n", str, \ + [[[NSError errorWithDomain:NSOSStatusErrorDomain code:err userInfo:nil] localizedDescription] UTF8String] \ + ) + static const int avf_time_base = 1000000; static const AVRational avf_time_base_q = { @@ -80,13 +85,12 @@ { AV_PIX_FMT_NONE, 0 } }; +#define MAX_QUEUED_FRAMES 10 + typedef struct { AVClass* class; - int frames_captured; - int audio_frames_captured; - pthread_mutex_t frame_lock; id avf_delegate; id avf_audio_delegate; @@ -122,8 +126,8 @@ AVCaptureSession *capture_session; AVCaptureVideoDataOutput *video_output; AVCaptureAudioDataOutput *audio_output; - CMSampleBufferRef current_frame; - CMSampleBufferRef current_audio_frame; + CMSimpleQueueRef audio_frames_queue; + CMSimpleQueueRef video_frames_queue; AVCaptureDevice *observed_device; #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070 @@ -132,16 +136,6 @@ int observed_quit; } AVFContext; -static void lock_frames(AVFContext* ctx) -{ - pthread_mutex_lock(&ctx->frame_lock); -} - -static void unlock_frames(AVFContext* ctx) -{ - pthread_mutex_unlock(&ctx->frame_lock); -} - /** FrameReciever class - delegate for AVCaptureSession */ @interface AVFFrameReceiver : NSObject @@ -219,17 +213,13 @@ - (void) captureOutput:(AVCaptureOutput *)captureOutput didOutputSampleBuffer:(CMSampleBufferRef)videoFrame fromConnection:(AVCaptureConnection *)connection { - lock_frames(_context); + OSStatus ret = CMSimpleQueueEnqueue(_context->video_frames_queue, videoFrame); - if (_context->current_frame != nil) { - CFRelease(_context->current_frame); + if (ret != noErr) { + av_log_avfoundation_error(_context, "Error while queueing video frame", ret); } - _context->current_frame = (CMSampleBufferRef)CFRetain(videoFrame); - - unlock_frames(_context); - - ++_context->frames_captured; + CFRetain(videoFrame); } @end @@ -263,17 +253,13 @@ - (void) captureOutput:(AVCaptureOutput *)captureOutput didOutputSampleBuffer:(CMSampleBufferRef)audioFrame fromConnection:(AVCaptureConnection *)connection { - lock_frames(_context); + OSStatus ret = CMSimpleQueueEnqueue(_context->audio_frames_queue, audioFrame); - if (_context->current_audio_frame != nil) { - CFRelease(_context->current_audio_frame); + if (ret != noErr) { + av_log_avfoundation_error(_context, "Error while queueing audio frame", ret); } - _context->current_audio_frame = (CMSampleBufferRef)CFRetain(audioFrame); - - unlock_frames(_context); - - ++_context->audio_frames_captured; + CFRetain(audioFrame); } @end @@ -288,6 +274,30 @@ static void destroy_context(AVFContext* ctx) [ctx->avf_delegate release]; [ctx->avf_audio_delegate release]; + CMSampleBufferRef frame; + + if (ctx->video_frames_queue) { + frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); + while (frame) { + CFRelease(frame); + frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); + } + + CFRelease(ctx->video_frames_queue); + ctx->video_frames_queue = NULL; + } + + if (ctx->audio_frames_queue) { + frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); + while (frame) { + CFRelease(frame); + frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); + } + + CFRelease(ctx->audio_frames_queue); + ctx->audio_frames_queue = NULL; + } + ctx->capture_session = NULL; ctx->video_output = NULL; ctx->audio_output = NULL; @@ -298,12 +308,6 @@ static void destroy_context(AVFContext* ctx) AudioConverterDispose(ctx->audio_converter); ctx->audio_converter = NULL; } - - pthread_mutex_destroy(&ctx->frame_lock); - - if (ctx->current_frame) { - CFRelease(ctx->current_frame); - } } static void parse_device_name(AVFormatContext *s) @@ -631,18 +635,18 @@ static int get_video_config(AVFormatContext *s) } // Take stream info from the first frame. - while (ctx->frames_captured < 1) { + while (CMSimpleQueueGetCount(ctx->video_frames_queue) < 1) { CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.1, YES); } - lock_frames(ctx); + CMSampleBufferRef frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); ctx->video_stream_index = stream->index; avpriv_set_pts_info(stream, 64, 1, avf_time_base); - image_buffer = CMSampleBufferGetImageBuffer(ctx->current_frame); - block_buffer = CMSampleBufferGetDataBuffer(ctx->current_frame); + image_buffer = CMSampleBufferGetImageBuffer(frame); + block_buffer = CMSampleBufferGetDataBuffer(frame); if (image_buffer) { image_buffer_size = CVImageBufferGetEncodedSize(image_buffer); @@ -658,10 +662,7 @@ static int get_video_config(AVFormatContext *s) stream->codecpar->format = ctx->pixel_format; } - CFRelease(ctx->current_frame); - ctx->current_frame = nil; - - unlock_frames(ctx); + CFRelease(frame); return 0; } @@ -681,27 +682,27 @@ static int get_audio_config(AVFormatContext *s) } // Take stream info from the first frame. - while (ctx->audio_frames_captured < 1) { + while (CMSimpleQueueGetCount(ctx->audio_frames_queue) < 1) { CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.1, YES); } - lock_frames(ctx); + CMSampleBufferRef frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); ctx->audio_stream_index = stream->index; avpriv_set_pts_info(stream, 64, 1, avf_time_base); - format_desc = CMSampleBufferGetFormatDescription(ctx->current_audio_frame); + format_desc = CMSampleBufferGetFormatDescription(frame); const AudioStreamBasicDescription *input_format = CMAudioFormatDescriptionGetStreamBasicDescription(format_desc); if (!input_format) { - unlock_frames(ctx); + CFRelease(frame); av_log(s, AV_LOG_ERROR, "audio format not available\n"); return 1; } if (input_format->mFormatID != kAudioFormatLinearPCM) { - unlock_frames(ctx); + CFRelease(frame); av_log(s, AV_LOG_ERROR, "only PCM audio format are supported at the moment\n"); return 1; } @@ -781,16 +782,13 @@ static int get_audio_config(AVFormatContext *s) if (must_convert) { OSStatus ret = AudioConverterNew(input_format, &output_format, &ctx->audio_converter); if (ret != noErr) { - unlock_frames(ctx); - av_log(s, AV_LOG_ERROR, "Error while allocating audio converter\n"); + CFRelease(frame); + av_log_avfoundation_error(s, "error while creating audio converter", ret); return 1; } } - CFRelease(ctx->current_audio_frame); - ctx->current_audio_frame = nil; - - unlock_frames(ctx); + CFRelease(frame); return 0; } @@ -808,8 +806,6 @@ static int avf_read_header(AVFormatContext *s) ctx->num_video_devices = [devices count] + [devices_muxed count]; - pthread_mutex_init(&ctx->frame_lock, NULL); - #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070 CGGetActiveDisplayList(0, NULL, &num_screens); #endif @@ -1010,6 +1006,21 @@ static int avf_read_header(AVFormatContext *s) // Initialize capture session ctx->capture_session = [[AVCaptureSession alloc] init]; + OSStatus ret; + ret = CMSimpleQueueCreate(kCFAllocatorDefault, MAX_QUEUED_FRAMES, &ctx->video_frames_queue); + + if (ret != noErr) { + av_log_avfoundation_error(s, "error while creating frame queue", ret); + goto fail; + } + + ret = CMSimpleQueueCreate(kCFAllocatorDefault, MAX_QUEUED_FRAMES, &ctx->audio_frames_queue); + + if (ret != noErr) { + av_log_avfoundation_error(s, "error while creating frame queue", ret); + goto fail; + } + if (video_device && add_video_device(s, video_device)) { goto fail; } @@ -1039,7 +1050,8 @@ static int avf_read_header(AVFormatContext *s) fail: [pool release]; destroy_context(ctx); - return AVERROR(EIO); + av_log(s, AV_LOG_ERROR, "Error while opening AVfoundation capture session\n"); + return AVERROR_EXTERNAL; } static int copy_cvpixelbuffer(AVFormatContext *s, @@ -1088,38 +1100,35 @@ static int copy_cvpixelbuffer(AVFormatContext *s, static int avf_read_packet(AVFormatContext *s, AVPacket *pkt) { OSStatus ret; + int status; AVFContext* ctx = (AVFContext*)s->priv_data; do { - CVImageBufferRef image_buffer; - CMBlockBufferRef block_buffer; - lock_frames(ctx); - - if (ctx->current_frame != nil) { - int status; + if (1 <= CMSimpleQueueGetCount(ctx->video_frames_queue)) { int length = 0; - - image_buffer = CMSampleBufferGetImageBuffer(ctx->current_frame); - block_buffer = CMSampleBufferGetDataBuffer(ctx->current_frame); + CMSampleBufferRef video_frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue); + CVImageBufferRef image_buffer = CMSampleBufferGetImageBuffer(video_frame);; + CMBlockBufferRef block_buffer = CMSampleBufferGetDataBuffer(video_frame); if (image_buffer != nil) { length = (int)CVPixelBufferGetDataSize(image_buffer); } else if (block_buffer != nil) { length = (int)CMBlockBufferGetDataLength(block_buffer); } else { - unlock_frames(ctx); + CFRelease(video_frame); return AVERROR(EINVAL); } - if (av_new_packet(pkt, length) < 0) { - unlock_frames(ctx); - return AVERROR(EIO); + status = av_new_packet(pkt, length); + if (status < 0) { + CFRelease(video_frame); + return status; } CMItemCount count; CMSampleTimingInfo timing_info; - if (CMSampleBufferGetOutputSampleTimingInfoArray(ctx->current_frame, 1, &timing_info, &count) == noErr) { + if (CMSampleBufferGetOutputSampleTimingInfoArray(video_frame, 1, &timing_info, &count) == noErr) { AVRational timebase_q = av_make_q(1, timing_info.presentationTimeStamp.timescale); pkt->pts = pkt->dts = av_rescale_q(timing_info.presentationTimeStamp.value, timebase_q, avf_time_base_q); } @@ -1133,18 +1142,18 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt) status = 0; ret = CMBlockBufferCopyDataBytes(block_buffer, 0, pkt->size, pkt->data); if (ret != kCMBlockBufferNoErr) { - status = AVERROR(EIO); + av_log_avfoundation_error(s, "error while copying buffer data", ret); + status = AVERROR_EXTERNAL; } } - CFRelease(ctx->current_frame); - ctx->current_frame = nil; + CFRelease(video_frame); if (status < 0) { - unlock_frames(ctx); return status; } - } else if (ctx->current_audio_frame != nil) { - CMBlockBufferRef block_buffer = CMSampleBufferGetDataBuffer(ctx->current_audio_frame); + } else if (1 <= CMSimpleQueueGetCount(ctx->audio_frames_queue)) { + CMSampleBufferRef audio_frame = (CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue); + CMBlockBufferRef block_buffer = CMSampleBufferGetDataBuffer(audio_frame); size_t input_size = CMBlockBufferGetDataLength(block_buffer); int buffer_size = input_size / ctx->audio_buffers; @@ -1170,8 +1179,9 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt) if (ret != kCMBlockBufferNoErr) { av_free(input_buffer); - unlock_frames(ctx); - return AVERROR(EIO); + CFRelease(audio_frame); + av_log_avfoundation_error(s, "error while accessing audio buffer data", ret); + return AVERROR_EXTERNAL; } } @@ -1188,23 +1198,25 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt) av_free(input_buffer); if (ret != noErr) { - unlock_frames(ctx); - return AVERROR(EIO); + CFRelease(audio_frame); + av_log_avfoundation_error(s, "error while converting audio data", ret); + return AVERROR_EXTERNAL; } pkt->size = output_buffer.mBuffers[0].mDataByteSize; } else { ret = CMBlockBufferCopyDataBytes(block_buffer, 0, pkt->size, pkt->data); if (ret != kCMBlockBufferNoErr) { - unlock_frames(ctx); - return AVERROR(EIO); + CFRelease(audio_frame); + av_log_avfoundation_error(s, "error while copying audio data", ret); + return AVERROR_EXTERNAL; } } CMItemCount count; CMSampleTimingInfo timing_info; - if (CMSampleBufferGetOutputSampleTimingInfoArray(ctx->current_audio_frame, 1, &timing_info, &count) == noErr) { + if (CMSampleBufferGetOutputSampleTimingInfoArray(audio_frame, 1, &timing_info, &count) == noErr) { AVRational timebase_q = av_make_q(1, timing_info.presentationTimeStamp.timescale); pkt->pts = pkt->dts = av_rescale_q(timing_info.presentationTimeStamp.value, timebase_q, avf_time_base_q); } @@ -1212,21 +1224,15 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt) pkt->stream_index = ctx->audio_stream_index; pkt->flags |= AV_PKT_FLAG_KEY; - CFRelease(ctx->current_audio_frame); - ctx->current_audio_frame = nil; - - unlock_frames(ctx); + CFRelease(audio_frame); } else { pkt->data = NULL; - unlock_frames(ctx); if (ctx->observed_quit) { return AVERROR_EOF; } else { return AVERROR(EAGAIN); } } - - unlock_frames(ctx); } while (!pkt->data); return 0;
* Use a CMSimpleQueueEnqueue with maximum length to queue and process incoming audio and video frames. * Log avfoundation errors. * Use AVERROR_EXTERNAL instead of AVERROR(EIO) in avfoundation errors. Signed-off-by: Romain Beauxis <toots@rastageeks.org> — [Sorry for the noise but an issue came up with the previous set] This is the second patch of a series of 3 that fix, cleanup and enhance the avfoundation implementation for libavdevice. These patches come from an actual user-facing application relying on libavdevice’s implementation of avfoundation audio input. Without them, Avfoundation is practically unusable as it will: * Refuse to process certain specific audio input format that are actually returned by the OS for some users (packed PCM audio) * Drop audio frames, resulting in corrupted audio input. This might have been unnoticed with video frames but this makes avfoundation essentially unusable for audio. The patches are now being included in our production build so they are tested and usable in production. Changelog for this patch: * v2: None * v3: None * v4: None * v5: Fix indentation/wrapping * v6: None * v7: Removed use of kAudioConverterPropertyCalculateOutputBufferSize to calculate output buffer size. The calculation is trivial and this call was randomly failing for no reason * v8: Fix memory leak when video or audio queue is full libavdevice/avfoundation.m | 194 +++++++++++++++++++------------------ 1 file changed, 100 insertions(+), 94 deletions(-)