[vlc-devel] [PATCH 1/3] qtsound: added audio capture functionality for MAC OS X

[Jean-Baptiste Kempf]

On Thu, Sep 22, 2011 at 08:33:04PM +0200, Michael Feurstein wrote :
> +* qtcapture.m: qtkit (Mac OS X) based capture module
qtsound.m

> +* Copyright (C) 2008 the VideoLAN team
Wrong copyright date.

> +* Authors: Pierre d'Herbemont <pdherbemont at videolan.org>
Wrong author.

> +*****************************************************************************
> +* This library is free software; you can redistribute it and/or
> +* modify it under the terms of the GNU Lesser General Public
> +* License as published by the Free Software Foundation;
> +* version 2 of the License.
You should use LGPLv2.1 or later more than LGPLv2.

> +/*****************************************************************************
> + * QTKit Bridge
> + *****************************************************************************/
> + at interface VLCDecompressedAudioOutput : QTCaptureDecompressedAudioOutput
> +{
> +	AudioBuffer *currentAudioBuffer;
> +	int lenghtForAllSamples, numberOfSamples;
No tabs in source code.

> +		if (currentAudioBuffer) 
> +		{
> +			free(currentAudioBuffer);
> +			currentAudioBuffer = nil;
> +		}
You don't need to test to free.

> +	int count = 0;
This should be UInt32 to match the tempAudioBufferList->mNumberBuffers
type.

> +			for ( count = 0; count < tempAudioBufferList->mNumberBuffers; count++ ) 
> +			{
> +				audioDataSize += tempAudioBufferList->mBuffers[count].mDataByteSize;
> +			}
> +			rawAudioData = malloc(audioDataSize * 2);
Check malloc return.

> +			int i;
You should use an unsigned short here, I think.

> +			/* mono (built-in mic, built-in input)
> +			 tested with 1.2-git version - works
Useless comment.

> +			/* this one works with griffin iMic which is stereo
> +			 tested with 1.1.11 source ball - works
> +			 tested with 1.2-git - seems broken
> +			 */
Why is it broken?

> +			/*
> +			float *b1Ptr, *b2Ptr;
> +			signed short *uPtr;
> +
> +			for (i = 0,
> +				 uPtr = (signed short *)rawAudioData,
> +				 b1Ptr = (float *) tempAudioBufferList->mBuffers[0].mData,
> +				 b2Ptr = (float *) tempAudioBufferList->mBuffers[1].mData;
> +				 i < 512; i++) 
> +			 {
> +				*uPtr = (signed short)(round((*b1Ptr) * 32767.00));
> +				uPtr ++;
> +				b1Ptr ++;
> +				*uPtr = (signed short)(round((*b2Ptr) * 32767.00));
> +				uPtr ++;
> +				b2Ptr ++;
> +			}
> +			 */


> +			if (currentAudioBuffer == nil) 
Trailing space (in addition to tabs)

> +	if(currentAudioBuffer->mData)
> +		return currentPts;
> +	else
> +		return 0;
return (currentAudioBuffer->mData) ? currentPts : 0;
maybe?

> +struct demux_sys_t {
> +    QTCaptureSession * session;
> +	QTCaptureDevice * audiodevice;
> +	VLCDecompressedAudioOutput * audiooutput;
> +	int i_audio_max_buffer_size;
> +	block_t *p_block_audio;
> +	es_out_id_t *p_es_audio;
You should put the int at the end, IMHO

> +static int Open( vlc_object_t *p_this )
> +{
> +    demux_t     *p_demux = (demux_t*)p_this;
> +    demux_sys_t *p_sys = NULL;
Useless init;

> +	es_format_t audiofmt;
> +    int i;
Where is i used?

> +	if(p_demux->psz_location && *p_demux->psz_location ){
Inconsistent spacing

> +    /* Set up p_demux */
> +    p_demux->pf_demux = Demux;
> +    p_demux->pf_control = Control;
> +    p_demux->info.i_update = 0;
> +    p_demux->info.i_title = 0;
> +    p_demux->info.i_seekpoint = 0;
You should assign the pointers and all the rest after the calloc.

> +    /* Get the formats */
> +	/*
> +	 FIXME: the format description gathered here does not seem to be the same when having started the QTCaptureSession. This information needs to be updated some other place.
> +	 [0x2f7068] qtcapture demux debug: Audio localized format summary: Linear PCM, 16 bit little-endian signed integer, 2 channels, 44100 Hz
> +	 [0x2f7068] qtcapture demux debug: Sample Rate: 44100.000000; Format ID: lpcm; Format Flags: 0000000c; Bytes per Packet: 4; Frames per Packet: 1; Bytes per Frame: 4; Channels per Frame: 2; Bits per Channel: 16
> +	 [0x2f7068] qtcapture demux debug: Flag float 0 bigEndian 0 signedInt 1 packed 1 alignedHigh 0 non interleaved 0 non mixable 0
> +	 canonical 1 nativeFloatPacked 1 nativeEndian 0
> +
> +	 should be for iMic
> +
> +	 number of samples in samplebuffer: 512
> +	 Audio localized format summary: Linear PCM, 32 bit little-endian floating point, 2 channels, 44100 Hz
> +	 Sample Rate: 44100; Format ID: lpcm; Format Flags: 00000029; Bytes per Packet: 4; Frames per Packet: 1; Bytes per Frame: 4; Channels per Frame: 2; Bits per Channel: 32
> +	 Flag float 1 bigEndian 0 signedInt 0 packed 1 alignedHigh 0 non interleaved 1 non mixable 0
> +	 canonical 1 nativeFloatPacked 1 nativeEndian 0
> +	 */

This comment must be improved.

> +		msg_Dbg( p_demux, "Sample Rate: %.0lf; Format ID: %s; Format Flags: %.8x; Bytes per Packet: %d; Frames per Packet: %d; Bytes per Frame: %d; Channels per Frame: %d; Bits per Channel: %d", 
> +				asbd.mSampleRate,
> +				formatIDString,
> +				asbd.mFormatFlags,
> +				asbd.mBytesPerPacket,
> +				asbd.mFramesPerPacket,
> +				asbd.mBytesPerFrame,
> +				asbd.mChannelsPerFrame,
> +				asbd.mBitsPerChannel);
> +
> +		msg_Dbg( p_demux, "Flag float %d bigEndian %d signedInt %d packed %d alignedHigh %d non interleaved %d non mixable %d\ncanonical %d nativeFloatPacked %d nativeEndian %d", 
> +				(asbd.mFormatFlags & kAudioFormatFlagIsFloat) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagIsBigEndian) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagIsSignedInteger) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagIsPacked) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagIsAlignedHigh) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagIsNonInterleaved) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagIsNonMixable) != 0,
> +
> +
> +				(asbd.mFormatFlags & kAudioFormatFlagsCanonical) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagsNativeFloatPacked) != 0,
> +				(asbd.mFormatFlags & kAudioFormatFlagsNativeEndian) != 0
> +				);

Those two should just be displayed in debug mode.

> +	int audiocodec = VLC_CODEC_S16L;
> +	//int audiocodec = VLC_CODEC_F32L;
Why, btw?

Best Regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device