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

proposal: Add an x/media package #16353

Open
rfliam opened this issue Jul 13, 2016 · 18 comments

Comments

@rfliam
Copy link

commented Jul 13, 2016

The Proposal

Golang has an x/image "extension" library. Adding an x/media package for parsing video and audio transports (and possibly codecs) seems a logical extension. Interacting with media is an extremely common task today, having this as part of the standard library would probably be beneficial for all.

Proposed Implementation

We at Comcast have recently open sourced our aac parser. We also are looking to open source our parsers for h.264, h.265, DD, DD+, and MPEG2-TS. We would like to work with the go community to make them available in a golang extensions package. These parsers are currently only for the framing and transports, not the actual decoding.

Similar Items

It appears the x/image has some ability to deal with the vp8 codec, but doesn't appear to deal with framing of any sort.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 13, 2016

What's wrong with putting them all on github?

I don't think anybody on the Go team has enough cycles to maintain & review so much code.

We don't want to be in a situation where there's a pile of code added to x/ and then the original authors leave Comcast and nobody at Comcast or on the Go team or the open source community knows how to maintain the code, but it has a blessed name under golang.org/x/....

In any case, we'd need to see the code first (h.264, h.265, DD, DD+, and MPEG2-TS) before we'd put it in x/. It would also have to match the Go style (golang.org/s/style) which the https://github.com/Comcast/gaad code doesn't match yet.

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2016

+1 to the idea of having x/media, but I agree with @bradfitz, I think it should wait until we have mergable code to place in the repo.

@rfliam

This comment has been minimized.

Copy link
Author

commented Jul 13, 2016

Certainly nothing is wrong with placing them on github.

However, the thrust of this proposal is that interacting with media is a common enough operation that having it available as part of x would be valuable. While I possibly have a biased view, interaction with media seems at least as common as other items

Obviously any contributed code would have to be carefully reviewed and brought to match all contributing guidelines.

We don't want to be in a situation where there's a pile of code added to x/ and then the original authors leave Comcast and nobody at Comcast or on the Go team or the open source community knows how to maintain the code, but it has a blessed name under golang.org/x/....

This objection would stand for any large contribution added by folks other than the "core" Go team. The same could be said for recent IBM z architecture addition...

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2016

My opinion is that parsing media files is equivalent to parsing image files; if we're going to have x/image in the repo, we should also have x/media. (Obviously, this requires that the code meet the coding standards of the Go project.)

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 13, 2016

If we have image, we might as well have media. And because media has audio, so we might as well have audio. And if we have packages for sight and sound, we might as well have smell, ....

It's a slippery slope.

We don't want to put everything in the world in x/ and have to draw the line somewhere.

IBM complied with the http://golang.org/wiki/PortingPolicy for the Z port.

The closest we have for a policy for package additions is https://golang.org/doc/faq#x_in_std which I encourage everybody to read.

There's an ongoing maintenance cost for any new code in x/ (it's run under our continuous build infrastructure, etc) and we're stretched thin as is.

Let's discuss further once there are more media packages.

@rakyll

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

@rfliam, there is an existing work on audio and the initial step is to merge core audio interfaces that will represent PCM data and encoders/decoders.

The proposal is at #13432 and we don't want to progress on any official package without having consensus on the core interfaces.

If you would like to contribute to the audio work, I'd suggest you to look at the proposal and try to implement an AAC decoder/encoder (with the parser bits) and give us feedback.

@rfliam

This comment has been minimized.

Copy link
Author

commented Jul 15, 2016

@rakyll Thanks for the heads up!

Your proposal mentions:

TODO(jbd): Proposal should also mention how the decoders will be organized. e.g. image package's support for png, jpeg, gif, etc decoders.

Even a small subset of common decoders will represent a large set of code. It seems to me adding an x/media package would be prudent. This also exposes these decoders and parsers for use outside the mobile project without requiring other dependencies.

If people would prefer we can move discussion to that proposal. However, it seems more focused on providing playback API's on mobile. I had hoped to add generic media parsing operations for all go users that such a library could then rely on.

@rakyll

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

@rfliam, nothing on the x/mobile audio proposal is actually mobile specific and I am not willing for us to end up with different representations for audio.

These interfaces and a big majority of the encoders/decoders should probably live outside of x/mobile. Hence, I am interested in your proposal and x/media.

@crawshaw had a few concerns about originally suggesting that location when we are initially working on the proposal. The x packages need to be maintained and there is not a single person who can work on full-time. Our ideas seem to be drafty at the moment and we are moving slowly. Promising an official package is a big responsibility given the quality of packages from the Go project.

A side note, I am happy with renaming the proposal to be about generic audio if everyone agrees there is no good reason why it is not so.

@rfliam

This comment has been minimized.

Copy link
Author

commented Jul 15, 2016

@rakyll Agreed with all the above points and concerns. My only caveat would be possibly discussing a package for all media decoders and parsers? Though perhaps limiting scope to just audio with structure for other elements would be useful.

Also agreed with renaming.

We have also just open sourced our go mpegts parser. This, other than some overly brief documentation, should match the go style guide. The mobile audio proposal includes HLS, which is generally carried in TS so I suspect it may be of some use for that as well.

@quentinmit quentinmit added this to the Proposal milestone Jul 29, 2016

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2016

I think the repo should be named x/media even if it only starts with audio support.

@as

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2016

@quentinmit

For the naming, I don't like "media", it feels imprecise compared to the usual names of go packages. Media feels like a directory with nothing but other packages, and it would be confusing to me for the image package to live outside this directory if it existed.

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2016

@as:

Fair enough. Where would you propose putting the packages for:

  • MPEG transport stream
  • MPEG audio
  • MP4 (aka QuickTime)

Since container formats contain both video and audio, they don't really belong in a package called either. What about "x/av"? This mirrors e.g. libav from C.

@as

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2016

@quentinmit

I like "av" over "media." The only competing acronym I found is "anti-virus", but I wouldn't expect this to confuse people looking at x/* or stdlib.

@nigeltao

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

We have also just open sourced our go mpegts parser. This, other than some overly brief documentation, should match the go style guide.

Curiosity got the better of me, so I had a skim. I made some notes about Go style at Comcast/gots#18

@nigeltao

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

For the naming, I don't like "media", it feels imprecise compared to the usual names of go packages. Media feels like a directory with nothing but other packages, and it would be confusing to me for the image package to live outside this directory if it existed.

FWIW, the existing golang.org/x/image, golang.org/x/net, golang.org/x/sys and golang.org/x/text directories are nothing but other packages. There may be other examples. I didn't do an exhaustive search.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Nov 21, 2016

On hold until #17244 is figured out.

@mattetti

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2017

This is also related to this proposal #18497 suggesting an audio standard package. However, in the proposal I'm advocating to keep codecs out of the standard library but using a shared interface. @rfliam I'd love to work with you in seeing if my audio.Buffer proposal would work well for your aac decoder https://github.com/Comcast/gaad

Let me know what you think.

@wsc1

This comment has been minimized.

Copy link

commented Sep 14, 2018

There is also the option of working with zc in the context of the codec project.

This needs actual encoding and/or decoding and doesn't address metadata and the like. Also to my understanding running an aac decoder in a commercial context has some patent requirements see here. But zc supports 3rd party distributed codec implementations.

zc is also working toward better interop with github.com/go-audio audio.Buffer.

Lastly, I find the title "media" too large. To me, codecs are only one part of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.