Add speaker ID function #48
Conversation
| audio_events_config: Optional[AudioEventsConfig] = None, | ||
| ws_headers: Optional[dict] = None, | ||
| timeout: Optional[float] = None, | ||
| get_speakers: Optional[bool] = False, |
There was a problem hiding this comment.
I think get speakers should be enabled by default when speaker diarization is requested rather than having an additional parameter to enable it. The user who does't need it can easily ignore it and the user who needs it can capture it just like any other message using an event handler.
This makes get_speakers function redundant as you don't have to teach the user how to enable it and how to capture it, only how to capture it. And how to capture it is already demonstrated in the example.
There was a problem hiding this comment.
it depends what type of diarization is requested as it must be set as "speaker" or "channel_and_speaker" only for the speaker id to work. I am in the process of adding the get_speakers flag to our rt and batch transcribers, It will be part of the speaker_diarization_config. Note that enabling speaker id by default, whenever diarization is enabled has its pros and cons - e.g. customers who don't want them would still receive the speaker id messages, just saying. I have tried to turn all the speaker id related errors into warnings as not to disrupt normal transcription job, so I think it will be still safe to auto-enable it. However, the moment we miss something, there will be no way for customers to disable speaker id if diarization is enabled . Hmm, I’m leaning slightly against auto-enabling it, explicit configuration feels safer and more transparent (just my thoughts atm)? So there are indeed pros and cons to that.
There was a problem hiding this comment.
It's possible that I'm missing some context here but when requested SpeakersResult message is only sent once at the end of the session? So it shouldn't cause any issues..
But since you're updating the transcriber to be enabled via speaker_diarization_config we should hold off on merging this change
There was a problem hiding this comment.
I would say we don't want to auto-enable, as speaker IDs are a form of biometics and storing and handling them has legal consequences beyond our usual transcripts. There's a risk if it's auto-enabled, that customers would receive and even store biometrics without being aware of it, and that could possibly place us in an awkward position? My instinct would be in general that we should err on the side of forcing people to request biometrics rather than assume they want them.
Separately, @dln22 when will the get_speakers flag be added to speaker_diarization_config? Just wondering if we want to merge this in the meantime or whether it's better to hold off on the additional flag until it's in the transcriber?
There was a problem hiding this comment.
@TudorCRL My changes have been merged already, but not yet officially released. Will let you know once that happens.
TudorCRL
force-pushed
the
feature/speaker-id-function
branch
from
e98fefc to
f5131c8
Compare
No description provided.