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

Add metadata value for presence of video channel in video blobs #42431

Merged
merged 1 commit into from
Jun 20, 2021

Conversation

brenogazzola
Copy link
Contributor

Summary

Complement to #42392
Related to rails/marcel#57

While building video editing capabilities in our app, we've stumbled on a bit of an edge case. Android's voice recorder saves files as m4a but encodes with 3GP (ftyp3gp4), which is a container for both audio and video. Marcel recognizes it as video, and this ends up breaking our pipeline, which is expecting to be able to use video filters on any video file.

This PR adds video boolean entry to the video analyzer, to indicate whether the file has a video channel or not.

Other Information

If you believe this PR makes sense, it might be better for me to close it and add the code to #42392. I've only separated it because I was not sure it would be approved.

While audio felt like a good key for presence/absence of audio channel, I'm not so sure about a video key in a video file. Might be better them to video_channel and audio_channel I think (or maybe _stream).

@rafaelfranca
Copy link
Member

Hey @lfalcao. Why are you approving PRs in this repository?

@rafaelfranca
Copy link
Member

@brenogazzola can you rebase this PR?

@lfalcao
Copy link
Contributor

lfalcao commented Jun 10, 2021

Hey @lfalcao. Why are you approving PRs in this repository?

Hi, I thought that reviewing merges would help both core members and contributors. If that's not being helpful I stop, no problem 🙇

@rafaelfranca
Copy link
Member

Ah, sorry, that is fine, it is actually why I asked. It is helpful but I could not distinguish between a real user activity and a bot activity.

@brenogazzola
Copy link
Contributor Author

@rafaelfranca Done

@zzak zzak added the ready PRs ready to merge label Jun 10, 2021
@pixeltrix pixeltrix merged commit c39364f into rails:main Jun 20, 2021
@pixeltrix
Copy link
Contributor

@brenogazzola thanks 👍🏻

@brenogazzola brenogazzola deleted the activestorage/videochannel branch June 20, 2021 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants