Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aborting cleanly #27

Open
rjappleton opened this issue Mar 20, 2018 · 8 comments
Open

aborting cleanly #27

rjappleton opened this issue Mar 20, 2018 · 8 comments

Comments

@rjappleton
Copy link

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 ?

@dnewman-gpsw
Copy link
Collaborator

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.

@rjappleton
Copy link
Author

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.

@shekh
Copy link

shekh commented Mar 21, 2018

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

@dnewman-gpsw
Copy link
Collaborator

dnewman-gpsw commented Mar 21, 2018

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.

@shekh
Copy link

shekh commented Mar 21, 2018

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.

@rjappleton
Copy link
Author

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);

@shekh
Copy link

shekh commented Mar 21, 2018

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

@rjappleton
Copy link
Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants