Whisper is inconsistent with returning last segment
Author: hollance
Pull request: no
Labels: name: Discussion

### Feature request

When the input audio is cut off in the middle of a word, Whisper may not predict an ending timestamp. How we handle this differs between decoding using the tokenizer or using a pipeline. It's also different from how OpenAI handles this.

To see what happens, let's run Whisper:

```python
# load model
from transformers import AutoProcessor, WhisperForConditionalGeneration
processor = AutoProcessor.from_pretrained("openai/whisper-tiny")
model = WhisperForConditionalGeneration.from_pretrained("openai/whisper-tiny")
tokenizer = processor.tokenizer

# load data
from datasets import load_dataset
dataset = load_dataset(
    "hf-internal-testing/librispeech_asr_demo", "clean", split="validation"
)
dataset = dataset.sort("id")

# create example that reproduces the issue
import numpy as np
example1 = dataset[0]["audio"]["array"]
example2 = dataset[1]["audio"]["array"]
example3 = dataset[1]["audio"]["array"]
example = np.concatenate([example1, example2, example3]).astype(np.float32)
example = example[:200000]

# get input spectrogram
inputs = processor(example, sampling_rate=16000, return_tensors="pt")
input_features = inputs.input_features

# make prediction including timestamps
predicted_ids = model.generate(input_features, return_timestamps=True)
processor.decode(predicted_ids[0], decode_with_timestamps=True, output_offsets=True)
```

This outputs:

```python
{'text': "


<|0.00|> Mr. Quilter is the apostle of the middle classes, and we are glad to welcome his gospel.<|6.00|><|6.00|> Nor is Mr. Quilter's manner less interesting than his matter.<|11.00|><|11.00|> Nor is Mr. Quilter's
",
 'offsets': [{'text': ' Mr. Quilter is the apostle of the middle classes, and we are glad to welcome his gospel.',
   'timestamp': (0.0, 6.0)},
  {'text': " Nor is Mr. Quilter's manner less interesting than his matter.",
   'timestamp': (6.0, 11.0)}]}
```

Notice that the last segment is: ` Nor is Mr. Quilter's
`. However, there is no entry for this in the `"offsets"` array. This happens because in `tokenization_whisper.py` in `_compute_offsets` the last segment is skipped if it does not end with a timestamp token. The `"text"` output, however, does include that last segment.

OpenAI does the following:

```python
# load model
import whisper
model = whisper.load_model("tiny")

# load example as above...

# make prediction
result = model.transcribe(
    example,
    verbose=True,
    condition_on_previous_text=False,
)
```

This does include the final segment:

```text
[00:00.000 --> 00:06.000]  Mr. Quilter is the apostle of the middle classes, and we are glad to welcome his gospel.
[00:06.000 --> 00:11.000]  Nor is Mr. Quilter's manner less interesting than his matter.
[00:11.000 --> 00:13.000]  Norris, Mr. Quilters.
```

The reason the text is different than with our Whisper (`Nor is Mr. Quilter's` vs `Norris, Mr. Quilters.`), is that OpenAI detects that the last segment does not end with a timestamp and is therefore incomplete. It now "rewinds" to the last timestamp token and makes a new prediction from there. This new prediction can be different since the input spectrogram has essentially been shifted in time. Since now only one segment is returned, the OpenAI logic uses the start and end time from the audio as the timestamps for this final segment.

We can also use a `pipeline` to run Whisper:

```python
from transformers import pipeline
pipe = pipeline(task="automatic-speech-recognition", model="openai/whisper-tiny")
pipe(example, return_timestamps=True)
```

This outputs the following:

```python
{'text': " Mr. Quilter is the apostle of the middle classes, and we are glad to welcome his gospel. Nor is Mr. Quilter's manner less interesting than his matter. Nor is Mr. Quilter's",
 'chunks': [{'timestamp': (0.0, 6.0),
   'text': ' Mr. Quilter is the apostle of the middle classes, and we are glad to welcome his gospel.'},
  {'timestamp': (6.0, 11.0),
   'text': " Nor is Mr. Quilter's manner less interesting than his matter."},
  {'timestamp': (11.0, None), 'text': " Nor is Mr. Quilter's"}]}
```

Here the list containing the timestamps is named `"chunks"` instead of `"offsets"` but otherwise contains the same information. But now it does include the final segment. The ending timestamp is None. 

It also outputs a warning because the last segment doesn't have an ending timestamp (yes, WhisperTimeStampLogitsProcessor was used):

> "There was an error while processing timestamps, we haven't found a timestamp as last token. Was WhisperTimeStampLogitsProcessor used?"

Long story short: 

- The behavior of `tokenizer.decode(..., with_offsets=True)` is different from the pipeline with `return_timestamps=True`, and both are different from what OpenAI does (`None` instead of the actual ending timestamp, no rewinding). In addition, the pipeline does not include the timestamps in the text but `tokenizer.decode()` does.
- Is the `tokenizer.decode(..., with_offsets=True)` behavior a bug?
- The implementation of how the pipeline implements this "split up segments by timestamps" (`tokenizer._decode_asr`) is different from how the tokenizer implements it (`tokenizer._compute_offsets`). So we have two different implementations doing the same thing but with different results.
- Could we perhaps make this a bit more consistent? The pipeline calls the returned segments "chunks", but also uses this same term for splitting up the audio into partially overlapping 30-second slices. Very confusing. 

### Motivation

I'm currently adding word-level timestamps to Whisper: https://github.com/huggingface/transformers/pull/23205

In the OpenAI implementation these timestamps are added to the returned segments. Obviously if the last segment isn't being included, we can't add the word-level timestamps there. The word-level timestamps should also work in the pipeline.


### Your contribution

Rather than just submitting a PR to fix this issue, I'm opening this up for discussion to decide how we want to handle this, as it affects multiple pieces of an already complex system.

Comments:
Thanks for the super comprehensive write-up! For sure, let's discuss here what the best approach to fixing this issue is. Here are my thoughts:
1. It's better to return the last segment with an incomplete timestamp rather than dropping it completely - when decoding with timestamps, many users will look just at `"offsets"` to get the transcription + timestamps bundled together, and ignore the overall `"text"` (since they assume the text in `"offsets"` will be the same as in `"text"`). If we drop the last segment because it doesn't have a timestamp, we effectively miss a transcription chunk (as well as a timestamp). IMO it's better to return the transcription for this last offset even if it has a missing timestamp, so at least we return the correct transcription overall. So here I do agree that there is a 'bug' in the tokenizer and we should change its behaviour to mirror that of the processor. Happy to remove `tokenizer. _compute_offsets` in favour of `tokenizer._decode_asr` (and if this is not allowed because of breaking changes, then have it call `tokenizer._decode_asr` under the hood).
2. I'm not sure there's a clean way of doing the 'rewind' trick that OpenAI do: `transformers` is very distinct in it's three sequential stages of inference: feature extractor -> model -> tokenizer. OpenAI are a bit more liberal in going between model and tokenizer (e.g. with this 'rewind' trick and their decode with temperature fallback algorithm). Adding the 'rewind' trick to the pipeline method would add a lot of custom complexity that we probably want to avoid. What we can quite easily do is update the warning message to say something along the lines of `"audio is cut off in the middle of a word, Whisper did not predict an ending timestamp"` to at least inform users of why the last timestamp is missing.

Also cc @Narsil - would be interested in hearing your thoughts on this!

Agreed on uniformity in handling those "incomplete" things

1. `(start, None)` is the easiest imo. (I agree with @sanchit-gandhi basically).
2. `rewind` trick is dirty and cannot be done in pipelines. `pipelines` are stateless, and this is what enables orthogonal batching. OpenAI cannot do batching. We could have exactly their code too somewhere else, but not in the aforementionned locations.

Having predictable runtime is really important imo, and rewind trick is killing that. Also if the model is super bad (which can happen on random and  badly finetuned models) then you'll still have incomplete chunks.

For `chunks` we cannot change the output things because of backward compatibity.


Agreed that we should fix this to keep the final segment. I can add it to my list. 

Also agree that the rewinding trick isn't something we should do, as it interferes with the batching approach. Plus it's kind of a hack anyway.

Keeping a timestamp of `None` to mean "end of the input" is simple on our end, but it might be less convenient for users to interpret what time this actually corresponds to (since the input may have padding and so it's not necessarily the length of the input, and the user may not be able to easily figure out where the padding occurs).


>  it might be less convenient for users to interpret

Indeed, but the timestamp is supposed to be emitted by the model and correspond the actual end of speech.

There can be padding, but also just silent audio. Ideally it would be nice to output a sensible default, but here if the model doesn't give us a timestamp, then... well we cannot really do anything about it, and we just don't have the information, trying to recreate something is IMHO lying to the user.

For instance, there is nothing preventing the model from outputting timestamps that are out of order even though it wouldn't make sense. but if the model is doing it I think we should just translate what the model is saying, even if nonsensical (gladly this doesn't seem to be actually occurring in the real world)

> if the model doesn't give us a timestamp, then... well we cannot really do anything about it, and we just don't have the information, trying to recreate something is IMHO lying to the user.

Fair point. 😄 

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.