Enhance Logging in main.c

[yoursanonymous]

##Description
This PR standardizes the logging mechanism in main.c by introducing structured macros and replacing raw printf calls with LOG_INFO or LOG_ERROR for consistency.

To improve log readability, allow for easier filtering of error vs. info messages, and simplify future logging extensions.

[joseph-yiu]

A potential risk of this approach is macro name clash with other codes inside your project, especially when the project contains 3rd party codes. LOG_INFO and LOG_ERROR are commonly used and therefore it might be best to add prefix in the macro names.
Additionally, in large projects the macro definition might already present, so the usual practice is to do the define conditionally.
e.g.
#ifndef AUDIOMARK_LOG_INFO
#define AUDIOMARK_LOG_INFO(fmt, ...) printf("[INFO] " fmt "\n", ##VA_ARGS)
#endif
#ifndef AUDIOMARK_LOG_ERROR
#define AUDIOMARK_LOG_ERROR(fmt, ...) printf("[ERROR] " fmt "\n", ##VA_ARGS)
#endif

Thanks and regards,
Joseph

[yoursanonymous]

Thanks for the suggestion, Joseph

I have updated the logging macros to use an AUDIOMARK_ prefix and wrapped the definitions with #ifndef guards to avoid name clashes with other code.

Thanks again for the helpful review.

Best regards,
Vinayak

[ruuddw]

Hi Vinayak, Joseph, thanks for your suggestions to further improve this benchmark.
I agree that the direct calling of printf is not ideal. But it's not extensively used, and with the AUDIOMARK prefix the macro names become very long and impact readability of the code.

May I suggest an alternative solution? In other EEMBC/SPEC EG benchmarks, for portability "th_printf" is used instead of "printf". And it's part of the porting layer (ee_api.h) similar like other standard C functions used in the benchmark.
I think it would be better to use th_printf in this benchmark as well. I didn't do a full search, but printf is not only used in main, also in ee_audiomark.c.

[joseph-yiu]

Thanks for the suggestion. I am happy with "th_printf" too, especially that it is already available it in the API.
regards,
Joseph

[ruuddw]

Just to be clear, for AudioMark the "th_printf" is not in the ee_api.h. For other benchmarks it is, but here it would need to be added.

[joseph-yiu]

Ah, thanks for your clarification. Sorry for misunderstood your previous comment.
regards,
Joseph

[joseph-yiu]

Sorry for the delay... I finally have time to look into the details.

The th_printf function is a part of the EEMBC MITH (Multi-Instance Test Harness).
https://github.com/eembc/coremark-pro/blob/4832cc67b0926c7a80a4b7ce0ce00f4640ea6bec/mith/src/th_lib.c#L678
To keep things simple I would avoid adding MITH into AudioMark.
Instead, I could add th_printf in these files:

• https://github.com/eembc/audiomark/blob/main/ports/arm/th_api.c
• https://github.com/eembc/audiomark/blob/main/ports/barebones/th_api.c

The disadvantage is that if someone already have a vendor specific port that use their own th_api.c, they will have to update their own code before compiling with the new version.

Now the biggest issue is that printf is used in many AudioMark files.
It seems awkward to change just main.c, but it is hard to justify the effort to change all of the printf to use th_printf (or to use C macros as originally proposed).

So unless there are strong demand from SPEC customers I think I will put this suggestion on hold for now. Thanks for the suggestion though.
regards,
Joseph