Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

Reader is not getting reset on a AudioInputProcessor::executeStopCapture #30

Closed
simplistiq opened this issue Jun 19, 2017 · 10 comments
Closed

Comments

@simplistiq
Copy link

We are observing the following:

In AudioInputProcessor::executeStopCapture, the m_reader is not being reset. The SDK initialization sequence requires specifying the Attachment Readers for the AudioInputStream. They should be reset when the stopCapture( ) event happens, we can see the line of code (m_reader.reset()) but for some reason they are not been released. Hence we can send only those many recognize events that correlate to the number of readers defined for the AudioInputStream.

Thanks,
Ameya Pandit
Qualcomm Engineering
apandit@qti.qualcomm.com

@kencecka
Copy link
Contributor

Hi Ameya,

Are you keeping a shared_ptr to the reader saved somewhere? Typically, the AIP creates a reader from the AudioInputStream, saves a shared_ptr to the reader in m_reader, and passes a shared_ptr to the reader to ACL. At this point, there are two references to the reader. When a stopCapture() event happens, m_reader is closed and reset(), which means there is no one remaining reference to the reader (in ACL). ACL will continue to stream data from the reader until it sees that the reader has closed, so there is some amount of delay from the stopCapture() event to the actual deletion of the reader.

In a correctly working system, this means that you may need two readers to support back-to-back recognize() events. One reader is from the first recognize(), which may be finishing streaming, and the second reader is to start a new recognize(). The AIP does not allow multiple concurrent recognize() events, so there should never be any risk of using more than two readers.

A third reader is needed if you are using a Keyword Detector (KWD).

Are you seeing evidence that more than 2-3 readers are being used simultaneously? If so, can you provide more details on how you are testing this?

Ken

@simplistiq
Copy link
Author

Hi Ken,

Thank you very much for the response. The m_reader is not being reset on executeStopCapture(). You do not possibly see it because all the integration tests do a setup()/tearDown() sequence and never execute more than 3 recognize events back-to-back.

  1. This is the top level initialization in the App. Called once with maxReaders = 3
    size_t bufferSize = AudioInputStream::calculateBufferSize(nWords, wordSize, maxReaders);

  2. We get 3 recognize events and then we get error no readers are free. It has to be done with the disableReaderLocked() method as none of them are free.

We have got it to work with the following workaround in the inline Reader::close(....) method in Reader.h

//QC workaround to release readers.
m_bufferLayout->disableReaderLocked(m_id);

You are right, reset should release it but it appears readers are not being released on a reset. We do not keep any readers in the App context.

Regards.
Ameya

@kencecka
Copy link
Contributor

Hi Ameya,

It sounds like we have a bit of a mystery. We have some full end-to-end test examples set up internally, and are able to make many recognize() calls with maxReaders=2. There is something unique or unusual going on with your environment.

The workaround you are using is unsafe, because it makes that reader available for other threads to use when the original user (probably ACL) does not know and may still be trying to read from it.

We really need to get to the bottom of why you are leaking readers. Is there anything unsual about the platform you are testing this on (toolchain, custom STL library, etc)? Are you able to reproduce this problem on a different platform (preferably a standard compiler - gcc/clang - on a standard PC OS)?

If you can reproduce this in some simple test code that you can share, I'll be glad to review and try to reproduce the issue here to debug it further.

Ken

@simplistiq
Copy link
Author

I agree the Unit Tests pass, and I see that in the Unit Test code.

We are running it on Embedded Linux. Please note these are running on real platforms. We are not using those readers, our app does have any need to keep them. Putting the logs, I can see that the first reader is not disabled after the first sequence is complete.

-> Start Recognize
<- Stop Capture
<- Speak
-> Playback Started Event
-> Playback Ended Event

Does this sequence cause any issue?

@kencecka
Copy link
Contributor

Nothing about the sequence looks concerning. Which ASR Profile are you using for your AudioProvider? From what you show above, I presume it is either NEAR_FIELD or FAR_FIELD, which means AVS will send a StopCapture directive. That in turn will call AudioInputProcessor::executeStopCapture(), which will call m_reader->close() and m_reader.reset() With the reader marked as closed, the ACL thread which is reading from it will also release its shared_ptr, which should allow it to be deleted.

Could you add some code to log m_reader.use_count() to in executeStopCapture() prior to the m_reader->reset() call?

Ken

@dhpp
Copy link
Contributor

dhpp commented Jun 23, 2017

Hello Ameya,

Thanks for letting us know about this issue. Ken mentioned it to me, and I have a few additional thoughts to share.

It looks to me like the issue is with ACL, not with AudioInputProcessor. Ken is correct in that the shared_ptr is being properly reset in the executeStopCapture call, and the only reason the reader being pointed to is not actually being released must be due to something else in the system having another reference to the reader via shared_ptr.

That other thing is ACL. At a high level, the reader (shared_ptr) is wrapped up within an avsCommon::avs::MessageRequest, which flows as follows:

AVSConnectionManager::sendMessage() -> HTTP2MessageRouter::send() -> HTTP2Transport::send() -> HTTP2Stream.

It's within the HTTP2Stream that the shared_ptr (to the MessageRequest, which contains a shared_ptr to the attachmentReader) is persisted.

The stream represents and manages a single request / response interaction with AVS. In this case, it will be the Recognize Event being sent. If the stream does not complete correctly, then it will persist, thus persisting the shared_ptr to the attachmentReader.

Now, it is also possible that the AudioInputProcessor does not correctly close out the underlying SDS, which the attachmentReader wraps. Thus, it may be in a hung state through no fault of the ACL. Ken can comment further on this.

There is a related issue within ACL, which we will be shortly addressing, tracked here:
#21
where a stream is left in a hung state, if the connection is broken before the request / response has time to complete. This is not exactly the same thing, but might be causing your issue.

I am curious about replication - how often does this occur for you? Do you see any other error / hang trace in the logs when it occurs? Thanks.

David

@kencecka
Copy link
Contributor

Hi Ameya,

There was another issue reported which is similar to what you are describing. After reviewing the sample code for that issue, I noticed that it is using the TestMessageSender shim between AudioInputProcessor and AVSConnectionManager, and that class accumulates MessageRequests if not used correctly. I've discussed the issue in more detail in my response to that issue.

Can you check your code to see if you are using TestMessageSender, or any other shim code between AudioInputProcessor and AVSConnectionManager? Normally, you can connect these two components directly together without any shim, and they should work correctly. The TestMessageSender shim only exists for testing purposes.

Ken

@simplistiq
Copy link
Author

Ken, Let me check this today. Will get back to you. Thanks!!

@simplistiq
Copy link
Author

Your theory is right. We had taken the TestMessageSender code and I see that the messages were added in the queue. Removing that piece of code, now the issue is no longer observed.

Thanks for taking the time to look into this Ken.

Regards,
Ameya

@kencecka
Copy link
Contributor

Fantastic - thanks for confirming. Let us know if you have any further questions.

Ken

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

No branches or pull requests

3 participants