slhck / ffmpeg-normalize

Audio Normalization for Python/ffmpeg
MIT License
1.28k stars 118 forks source link

refactor: Improved logging behavior #216

Closed g3n35i5 closed 1 year ago

g3n35i5 commented 1 year ago

This PR improves the logging behavior of ffmpeg-normalize when used via API/python import.

With the current implementation, the logging configuration of ffmpeg-normalize overwrites the root logging behavior even if it's used via python import. For example, the (now deleted) section

if system() not in ("Windows", "cli"):
    logging.addLevelName(logging.ERROR, "ERROR")
    logging.addLevelName(logging.WARNING, "WARNING")
    logging.addLevelName(logging.INFO, "INFO")
    logging.addLevelName(logging.DEBUG, "DEBUG")

added the ANSI color codes to all log messages and the entire output of ffmpeg-normalize is put into the log of the application that imports it. However, in most cases the developers might want to configure this behavior.

With this implementation, you can choose whether or whether not to use the logging output of ffmpeg-normalize. To enable the log output, the following code section (or similar) is necessary:


# handler is defined by your application
ffmpeg_logger = logging.getLogger('ffmpeg_normalize')
ffmpeg_logger.addHandler(handler)
ffmpeg_logger.setLevel("DEBUG")

Please note that the CLI logging behavior of ffmpeg-normalize doesn't change with this PR. I'm configuring the logger in the CLI entrypoint in the same way as it was before (with the help of colorlog

slhck commented 1 year ago

Thanks! I will have a better look at this tomorrow. Logging had always felt a little bit bolted on, so I really appreciate your input. (In fact I may have to use the same approach in all other packages.)

g3n35i5 commented 1 year ago

Thanks! I will have a better look at this tomorrow. Logging had always felt a little bit bolted on, so I really appreciate your input. (In fact I may have to use the same approach in all other packages.)

Glad that I can help! There is certainly still some what you could change / improve the logging. I wanted to make only a minimal improvement with this PR, which does not bring too many changes with it.

slhck commented 1 year ago

Merged and released as 1.26.2!

slhck commented 1 year ago

Unfortuantely this has broken conda-forge builds: https://github.com/conda-forge/ffmpeg-normalize-feedstock/pull/17

I think I can fix this with the requirements.txt adding the dependency.