Skip to content

Conversation

@kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Aug 11, 2022

Series of patches to add non-audio probe buffer support (and few minor fixes).

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 12, 2022

Converting to a draft while debugging a possible bug.

@kv2019i kv2019i force-pushed the topic/probe-app-nonaudio branch 2 times, most recently from 106cacb to 8c305b4 Compare August 12, 2022 07:29
@kv2019i kv2019i marked this pull request as ready for review August 12, 2022 07:30
@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 12, 2022

Ready for review. Turns out we also need to add support for non-aligned sync words and payloads. Added this as a separate patch, retested and uploaded.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 12, 2022

The checkpatch issues around memcpy and function tab levels, are present in existing code, and I did not want to start refactoring surrounding code in this PR.

Copy link
Member

@abonislawski abonislawski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good update to probe-app

Copy link
Contributor

@jsarha jsarha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return *((uint32_t *)buf) == PROBE_EXTRACT_SYNC_WORD; or even...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love C. I guess here both memset(&data...) and memset(data...) would work and mean the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh Well, true, but this isn't really part of my PR. If you change the style, you should change across the file and better done in a separate patch.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here - and subjectively fread(data,...) seems clearer to me, but it's just me

@kv2019i kv2019i marked this pull request as draft August 15, 2022 13:39
@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 15, 2022

@lyakh found a bug in finalize wave header function. Turning to draft to block merge until bug is fixed.

@kv2019i kv2019i force-pushed the topic/probe-app-nonaudio branch from 8c305b4 to e7ab5c1 Compare August 16, 2022 11:30
@kv2019i kv2019i marked this pull request as ready for review August 16, 2022 11:31
Don't assume buffer-id of zero is an invalid value.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Do not blindly call fwrite without checking whether file open
succeeded or not.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Modify code to correctly handle probe streams where sync word
can occur at any byte boundary, and where probe packet size may
not be aligned to 32bit word size.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Identify non-audio probe streams and write them out with ".bin"
extension and without the RIFF WAVE header.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 16, 2022

Bug found by @lyakh fixed, pushed a new version. Ready for review and merge again.

@lgirdwood lgirdwood merged commit 87425ba into thesofproject:main Aug 16, 2022
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

Successfully merging this pull request may close these issues.

6 participants