cineform-sdk icon indicating copy to clipboard operation
cineform-sdk copied to clipboard

aborting cleanly

Open rjappleton opened this issue 6 years ago • 8 comments

First of all, thanks for releasing this to the community!

I've got the encoding working OK. I have 1 input thread submitting input samples by calling CFHD_EncodeAsyncSample(), and another output thread getting output samples by calling CFHD_WaitForSample(). Both of these functions (and therefore the threads) can block.

Perhaps I've missed something, but I can't find a way to cleanly abort if, for example, the user wants to prematurely cancel the encoding. At the moment of abort, either of the threads may be blocked. I've found that in a third thread I can call CFHD_ReleaseEncoderPool() - that unblocks both threads but with an exception, so this is obviously a dirty way of doing this that might leave the heap in a mess or a memory leak.

Is there a way of aborting cleanly, or just forcing CFHD_EncodeAsyncSample() and CFHD_WaitForSample() to unblock cleanly ?

rjappleton avatar Mar 20 '18 13:03 rjappleton

Does CFHD_StopEncoderPool() not work? I my quick test it seems to do what you need. It should finish any work in progress, but no new frames with be started.

dnewman-gpsw avatar Mar 20 '18 20:03 dnewman-gpsw

Thanks for the fast response.

CFHD_StopEncoderPool() just calls CEncoderPool::StopEncoders(). CFHD_ReleaseEncoderPool() destroys the encoder pool object whose destructor just calls StopEncoders()

In other words, StopEncoderPool gets called whether you call CFHD_StopEncoderPool() or CFHD_ReleaseEncoderPool().

For example, to simulate an extreme situation where the output thread might be busy doing something else, if I call CFHD_EncodeAsyncSample() repeatedly without any calls to CFHD_WaitForSample(), then the pool queue fills up and CFHD_EncodeAsyncSample() blocks. While it is blocked, from a third thread I call CFHD_StopEncoderPool() and nothing happens - the input thread is still blocked inside CFHD_EncodeAsyncSample(). If the third thread then calls CFHD_ReleaseEncoderPool() which deletes the CEncoderPool object, then there is a divide by zero exception in the first thread inside CFHD_EncodeAsyncSample().

At the moment I am handling this by catching the exception, but it seems a bit messy.

I agree that it would be great if CFHD_StopEncoderPool() was actually guaranteed to unblock CFHD_EncodeAsyncSample() and CFHD_WaitForSample() which would return a CFHD error code to indicate that the call was aborted. Perhaps it will cause future calls to be rejected (I haven't checked that), but it seems that CFHD_StopEncoderPool() will not force unblock existing calls.

rjappleton avatar Mar 21 '18 13:03 rjappleton

My understanding of safe abort (it seems to work ok in VirtualDub2):

  1. stop trying to send new frames for compression
  2. consume all compressed frames
  3. release objects Hope this helps

shekh avatar Mar 21 '18 13:03 shekh

Any recommendations?

In the sample code this situation doesn't come up as it only queues frames for encoding if there is slots in the queue.

if(queuedFrames < POOL_QUEUE_LENGTH)
    CFHD_EncodeAsyncSample(..), queuedFrames++;

if (queuedFrames)
     if (CFHD_ERROR_OKAY == CFHD_TestForSample(..))
         error = CFHD_GetEncodedSample(..), queuedFrames--;

This would never have the encoder pool block.

dnewman-gpsw avatar Mar 21 '18 19:03 dnewman-gpsw

In my code it is similar to example (single thread). I tried to repeat said extreme conditions and see what happens. I cannot identify any blocking in CFHD_EncodeAsyncSample(). However I got a deadlock in CFHD_ReleaseEncoderPool(). There is semaphore in CEncoderMessageQueue and it can overflow with 1024 messages (semaphore max count). Then new messages do not signal it.

shekh avatar Mar 21 '18 20:03 shekh

Consuming all compressed frames would be OK, but with separate in and out threads and a third thread wanting to do the abort, it is difficult to come up with a method which gives 100% confidence that the in and out threads will unblock in all situations.

To reproduce the "extreme" case I mentioned, just repeatedly call CFHD_EncodeAsyncSample() and do NOT call CFHD_WaitForSample() nor CFHD_TestForSample(). The queue will then fill up. Assuming you have set a queue length of say 8, then it will block in the 9th call to CFHD_EncodeAsyncSample(). The stack trace of the block is:

EncoderJobQueue::AddEncoderJob() CEncoderPool::EncodeSample() CFHD_EncodeAsyncSample()

I think it is blocking at the ConditionVariable:

// Wait until there is space in the queue space.Wait(mutex);

rjappleton avatar Mar 21 '18 22:03 rjappleton

Now I see this. Same idea as before: stop sending, continue consuming. Let the process finish naturally.

shekh avatar Mar 21 '18 22:03 shekh

Yes, as David mentioned that's fine when using a single thread, but when using separate input, output and control threads you really need a way of ensuring 100% that the functions can be forced to unblock.

For example in DirectShow an output thread can block in GetDeliveryBuffer() as it waits for an output buffer to become available. But when the filter graph is stopped/flushed, GetDeliveryBuffer() is FORCED to unblock and returns an error code. I think that something similar is needed in the Cineform SDK for both input and output functions.

I can cope with handling the exception for now. If I have some free time I''l try to find out how the unblocking this can be added, but I'm hampered from stepping through the SDK code by an obscure problem with the debug build of the SDK (the release build works fine).

rjappleton avatar Mar 22 '18 12:03 rjappleton