Skip to content

fix: transcribe verbosity#140

Merged
jongwook merged 1 commit intoopenai:mainfrom
nick-konovalchuk:fix-transcribe-verbosity
Sep 26, 2022
Merged

fix: transcribe verbosity#140
jongwook merged 1 commit intoopenai:mainfrom
nick-konovalchuk:fix-transcribe-verbosity

Conversation

@nick-konovalchuk
Copy link
Copy Markdown
Contributor

@nick-konovalchuk nick-konovalchuk commented Sep 26, 2022

I've hidden "Detected langauge: " massage behind verbose flag.
Also fixed the flag usage with tqdm

@jongwook
Copy link
Copy Markdown
Collaborator

I actually intended the two to be displayed even when --verbose is False, thinking it'd help the user notice any language detection errors and show the progress/ETA without flooding the console.

@nick-konovalchuk
Copy link
Copy Markdown
Contributor Author

nick-konovalchuk commented Sep 26, 2022

@jongwook well, ETA is disabled when verbose=True, which is misleading as it seem to me. Also, the language tag is returned as a part of the result. Do you really need to print it if you return it?

@jongwook
Copy link
Copy Markdown
Collaborator

jongwook commented Sep 26, 2022

The language tag is returned in the result, but for those calling whisper in the command line (as it seems to be a popular use case) the detail is lost.

Regarding ETA: that's true; it's not the most intuitively named options but I thought both formats have merits (like below) and thought it'd be nice to have these verbose/concise output options, but probably better not mix up those two.

image

We can maybe make it an integer level for verbosity if a totally silent output is desired by many ..

@nick-konovalchuk
Copy link
Copy Markdown
Contributor Author

nick-konovalchuk commented Sep 26, 2022

@jongwook
How about str verbosity like "full", "minimal" and "silent"? I might add it real quick

  • full being equivalent to verbosity=True
  • minimal being equivalent to verbosity=False
  • silent being equivalent to verbosity=False from current PR

@jongwook
Copy link
Copy Markdown
Collaborator

jongwook commented Sep 26, 2022

I'd like to avoid magic string if possible. It's a bit unorthodox but I'm inclined to making it an Optional[bool] and use None/False/True so verbose=None (i.e. silent) is the default in transcribe() but from the command line it selects either True or False.

I can make these edits on this PR if you'd like/don't mind.

@nick-konovalchuk
Copy link
Copy Markdown
Contributor Author

Don't bother yourself, I have nothing to do at the moment

previous_seek_value = seek

with tqdm.tqdm(total=num_frames, unit='frames', disable=verbose) as pbar:
with tqdm.tqdm(total=num_frames, unit='frames', disable=verbose is False) as pbar:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you make this verbose is not False? Thank you.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@jongwook jongwook merged commit b4308c4 into openai:main Sep 26, 2022
heejipark23 pushed a commit to heejipark23/whisper that referenced this pull request Sep 21, 2025
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.

2 participants