Skip to content

Conversation

@ankurvdev
Copy link
Contributor

While using llama-mtmd-cli some of the messages bleed into the output which prevents redirecting the actual output from the model into a file

This happens because the standard logging macros were not in sync with the rest of the project.

The fix makes sure the common log.h that defines the macros is used and removes the overridden declaration

Make sure to read the contributing guidelines before submitting a PR

@ankurvdev ankurvdev requested a review from ngxson as a code owner November 12, 2025 20:43
@ngxson
Copy link
Collaborator

ngxson commented Nov 12, 2025

mtmd_helper is not expected to linked against libcommon, we cannot include log.h here

@ngxson ngxson closed this Nov 12, 2025
@ankurvdev
Copy link
Contributor Author

Can you give me a little background as to why or point me to a discussion with the details.

The problem this PR solves is annoying enough that I'd like to fix it but not sure what the right fix here is without knowing what the underlying reason for avoiding libcommon are.

@ngxson
Copy link
Collaborator

ngxson commented Nov 12, 2025

mtmd-helper is suppose to be a demo to show how mtmd can interact directly with libllama. Linking against libcommon make it impossible to reuse the code in downstream projects.

@ankurvdev
Copy link
Contributor Author

Thanks for the context.

Wondering then if its ok to create a global logging_func variable in the mtmd library that defaults to fprintf but can be overriden to point to libcommon funcs inside the llama-mtmd-cli

The llama-mtmd-cli seems to link to libcommon so hopefully that should work.

Might be useful to other users of mtmd to plug in their loggers as well.

How does that sound?

@ankurvdev
Copy link
Contributor Author

ankurvdev commented Nov 13, 2025

I pushed an updated commit to show what I meant and check if this looks acceptable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants