[vlc-devel] commit: Clean up the code a bit, turning to be half as laggy as the previous code, but it will need to be properly fixed. ( Felix Paul Kühne )

[Pierre d'Herbemont]

On Jun 12, 2008, at 7:44 PM, Felix Paul Kühne wrote:

>> Any reason for that? No need to malloc() the p_sys if we can't probe
>> it... Not a big deal, but...
> That's because I'm using p_sys's capture objects (session, device, ..)
> directly without allocating temporary objects in Open(). Should be
> faster and less memory consuming, even if we need to allocate the
> p_sys first. The actual init is still done after successfully
> detecting an input device.

Well, it shouldn't be a big optimization IMO, but :)

>> ...This is wrong. In case of error you end up in a double free.
> Ah, that's true. I'll fix that later today.
>
>
>> No need to lock here. Why would you do that  
>> copyCurrentFrameToBuffer:?
>> already takes care of that.
> That's what I thought too, but apparently locking here and waiting for
> the p_block to be complete gives a speed up of about 25 to 35 pro
> cent, meaning less but still noticeable delay.

I won't mind a comment for that one. But I don't understand why at  
all... Any hints?

>
>>> -        currentPts = [sampleBuffer presentationTime].timeValue /
>>> [sampleBuffer presentationTime].timeScale;
>>> +        currentPts = [sampleBuffer presentationTime].timeValue;
> By using mtime_t for currentPts, we are loosing a lot of precision in
> the long run if we do this division here, so I removed it. This
> doesn't affect our time stamps at all actually because of the pts =
> mdate() later on the code, which should be removed too IMO.

ok, if you have better result.

> @@ -118,12 +117,13 @@ vlc_module_end();
>         if(!currentImageBuffer) return 0;
>         imageBuffer = CVBufferRetain(currentImageBuffer);
>         pts = currentPts;
> -    }
> +
>
>     CVPixelBufferLockBaseAddress(imageBuffer, 0);
>     void * pixels = CVPixelBufferGetBaseAddress(imageBuffer);
>     memcpy( buffer, pixels, CVPixelBufferGetBytesPerRow(imageBuffer)  
> * CVPixelBufferGetHeight(imageBuffer) );
>     CVPixelBufferUnlockBaseAddress(imageBuffer, 0);
> +    }

I didn't notice this one at first but I also don't understand it.  
Formatting is also incorrect then.

Pierre.