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 mp3 support to FLV extractor #2683

Merged
merged 2 commits into from
Apr 21, 2017
Merged

Added mp3 support to FLV extractor #2683

merged 2 commits into from
Apr 21, 2017

Conversation

tylerjroach
Copy link
Contributor

MP3 support has been added to the flv extractor. This will be more important if #2590 or #2503 is accepted for rtmp streams with mp3 audio. This commit will add mp3 audio support to both of those pull requests as well as standard flv playback.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@tylerjroach
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot
Copy link

CLAs look good, thanks!

@tylerjroach tylerjroach mentioned this pull request Apr 13, 2017
Copy link
Contributor

@ojw28 ojw28 left a comment

Choose a reason for hiding this comment

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

This change looks good! Please see the one comment inline and remove the dead code if you agree. Thanks!

if (audioFormat == AUDIO_FORMAT_ALAW || audioFormat == AUDIO_FORMAT_ULAW) {
if (audioFormat == AUDIO_FORMAT_MP3) {
int sampleRateIndex = (header >> 2) & 0x03;
if (sampleRateIndex < 0 || sampleRateIndex >= AUDIO_SAMPLING_RATE_TABLE.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it's impossible. Given L68, sampleRateIndex can only be 0,1,2 or 3 here, I think? If you agree, please delete this block (and the TAG constant near the top).

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@rkrishnan2012
Copy link
Contributor

I consent to my commits being used.

@ojw28 ojw28 merged commit 5381b4e into google:dev-v2 Apr 21, 2017
@google google locked and limited conversation to collaborators Aug 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants