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

Added AAC codec to mp4 #345

Merged
merged 1 commit into from
Mar 6, 2017
Merged

Conversation

jeromegrosse
Copy link
Contributor

Fix of the issue #344

@mbeacom
Copy link
Collaborator

mbeacom commented Jan 30, 2017

@jeromegrosse Thanks for the PR.

AAC is an formatting/codec type for Audio (this extensions dict defines video codec types. Just looking for some clarification.
What issue does this resolve?

@ping
Copy link

ping commented Jan 30, 2017

I'm not the OP, but the problem is specified in #344
Without this PR, I cannot specify aac as the audio codec in write_videofile()

vidclip.write_videofile(output_file, codec='libx264', audio=True, audio_codec='aac')

@ghost
Copy link

ghost commented Mar 1, 2017

@mbeacom, should we merge this pull request or should we look at adding an audio_codec to the dictionary?

@jeromegrosse
Copy link
Contributor Author

@mbeacom
The extention_dict dictionnary is used by find_extention which is itself used by write_videofile when looking for the right codec to use for the audio file. This PR allows the possibility to use AAC and MP4.

The way I understood it, extention_dict is used to instinctively look up the audio and video codecs. Maybe it is semantically wrong though, and we should use two different dictionary, one for video codecs and one for audio codecs?

@mbeacom
Copy link
Collaborator

mbeacom commented Mar 6, 2017

@jeromegrosse From the look of it, we should probably split up the dictionaries. This will require additional refactoring throughout the codebase to split up the dictionaries. @Earney Maybe we merge this and open a separate issue to track potentially splitting up the codec dictionaries?

@mbeacom mbeacom self-requested a review March 6, 2017 13:32
Copy link
Collaborator

@mbeacom mbeacom left a comment

Choose a reason for hiding this comment

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

:shipit: Looks good until we can refactor the codec dictionary handling.

@mbeacom mbeacom merged commit 407cbd7 into Zulko:master Mar 6, 2017
@ghost
Copy link

ghost commented Mar 6, 2017

will do.

@mbeacom mbeacom mentioned this pull request Mar 6, 2017
@jeromegrosse jeromegrosse deleted the enhancement-aac-mp4 branch March 6, 2017 15:44
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