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

✨ Reencode task for worker #59

Merged
merged 1 commit into from
May 15, 2023
Merged

✨ Reencode task for worker #59

merged 1 commit into from
May 15, 2023

Conversation

pajowu
Copy link
Member

@pajowu pajowu commented Apr 3, 2023

Reencode everything to 128k cbr mp3 and m4a

@pajowu pajowu requested review from phlmn, rroohhh and anuejn April 3, 2023 17:06
@anuejn
Copy link
Member

anuejn commented Apr 4, 2023

Do we really want to do this in the backend?.
I have the vague recollection, that we discussed to do it in the frontend, because it saves us compute and the users bandwidth.

@pajowu
Copy link
Member Author

pajowu commented Apr 4, 2023

Yes and no. I think it makes sense to do this in the frontend too, but I think we should have to option in the backend as well. a) to make sure the file is in the right format and b) to handle cases where the conversion in the frontend is not possible (e.g. files larger than 2GB, browsers without wasm)

Copy link
Member

@anuejn anuejn left a comment

Choose a reason for hiding this comment

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

I am fine with merging transcription at the worker since it is an improvement from the status quo :). However I would like to investigate transcoding at the frontend in the future.

worker/pyproject.toml Show resolved Hide resolved
@anuejn
Copy link
Member

anuejn commented Apr 4, 2023

fair points :)

@anuejn
Copy link
Member

anuejn commented Apr 4, 2023

Ah we probably also need a way to inform the frontend, that the audio file changed. -> Future

@pajowu pajowu marked this pull request as draft April 26, 2023 00:09
@pajowu pajowu mentioned this pull request Apr 28, 2023
@pajowu pajowu force-pushed the pajowu/reencode_worker branch 3 times, most recently from 5d35a14 to 801d3e8 Compare May 7, 2023 18:14
@pajowu pajowu marked this pull request as ready for review May 7, 2023 18:15
@pajowu pajowu requested a review from anuejn May 7, 2023 18:15
backend/transcribee_backend/routers/document.py Outdated Show resolved Hide resolved
backend/transcribee_backend/routers/document.py Outdated Show resolved Hide resolved
backend/transcribee_backend/routers/document.py Outdated Show resolved Hide resolved
@pajowu pajowu added this pull request to the merge queue May 15, 2023
Copy link
Member

@rroohhh rroohhh left a comment

Choose a reason for hiding this comment

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

load_audio should probably be moved to the ffmpeg-python package at some point aswell

Copy link
Member

@rroohhh rroohhh left a comment

Choose a reason for hiding this comment

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

load_audio should probably be moved to the ffmpeg-python package at some point aswell

Merged via the queue into main with commit b278b9b May 15, 2023
@pajowu pajowu deleted the pajowu/reencode_worker branch May 15, 2023 18:09
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.

3 participants