Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Never add video: prefix to transcription properties #1801

Closed
lfcnassif opened this issue Aug 6, 2023 · 5 comments
Closed

Never add video: prefix to transcription properties #1801

lfcnassif opened this issue Aug 6, 2023 · 5 comments
Assignees
Labels

Comments

@lfcnassif
Copy link
Member

lfcnassif commented Aug 6, 2023

Just tested transcription on videos simply adding video into mimesToProcess in AudioTranscriptConfig. I thought it would be necessary to update the convertCommand (and that's why I exposed it to interested users) but it wasn't needed (reported by @hauck-jvsh before).

But transcription results are being prefixed by video:. That's not good because it splits results between different properties and it also avoids to embed the video transcriptions into chats properly.

@lfcnassif lfcnassif self-assigned this Aug 6, 2023
@lfcnassif
Copy link
Member Author

lfcnassif commented Aug 6, 2023

Tagging as enhancement since transcribing videos wasn't the original intent.

@lfcnassif lfcnassif changed the title Avoid adding video: prefix to video transcription results Avoid adding video: prefix to optional video transcriptions Aug 6, 2023
@hauck-jvsh
Copy link
Member

hauck-jvsh commented Aug 6, 2023

I don't know if it is a good idea to expose this, as it will send the entire video file, which maybe large, over the network. In this case I think that the audio should be extracted in the client.

@wladimirleite
Copy link
Member

I agree. For videos, running MPlayer locally before sending to transcription server makes more sense.

Another detail, maybe the conversion command can be slightly changed to improve performance, completely ignoring the video channel. Not sure if it already does that. I will check this later and update here.

@lfcnassif
Copy link
Member Author

Thanks @hauck-jvsh, I agree, but the option was already exposed, user was already able to change the configuration, I just fixed the property name. Let's open another ticket to improve video transcription resource usage.

Thanks @tc-wleite, let us know if the command can be improved.

@lfcnassif
Copy link
Member Author

I changed my mind and will tag this as bug.

@lfcnassif lfcnassif added bug and removed enhancement labels Aug 6, 2023
@lfcnassif lfcnassif changed the title Avoid adding video: prefix to optional video transcriptions Never add video: prefix to transcription properties Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants