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

[for discussion] proposal for a Video atom #103

Closed
wants to merge 1 commit into from

Conversation

akash1810
Copy link
Member

@akash1810 akash1810 commented Aug 7, 2017

The media atom has evolved over time and as a result its not entirely obvious what's what, for example, there's a posterUrl field and a posterImage field, there's a Metadata struct which primarily holds youtube metadata, however it also has PlutoData.

Furthermore, the media atom has been designed for audio and video across multiple platforms. Its unlikely we'll get the opportunity to realising these initial plans. Additionally, I don't think we'll ever publish an atom to the facebook, dailymotion or mainstream platforms, primarily because the content on these platforms is often a different version of the content used on site. As a result, the Platform enum is a lot simpler.

This is a proposal for a video atom definition. There's no intention of using it in production, its more for discussion... unless of course its beneficial to migrate atoms...

@regiskuckaertz
Copy link

I think that's sensible to separate audio and video. FWIW, the library we are working on is designed to handle as many rendering surfaces as necessary. In other words, the same atom can be rendered differently depending on the context (facebook et al) and there is no need for any rendering code anywhere anymore (composer, atom-workshop, frontend, or even the defaultHtml field that we will eventually deprecate)

@akash1810
Copy link
Member Author

In other words, the same atom can be rendered differently depending on the context (facebook et al) and there is no need for any rendering code anywhere anymore (composer, atom-workshop, frontend, or even the defaultHtml field that we will eventually deprecate)

YESSSS!!!! 👍

I mentioned Facebook etc as the original intention of the media atom was to upload content to those platforms. This has since evolved into a similar pattern we employed before, that is, Audience recut the video and upload to FB natively. For example, this video on theguardian.com is here on facebook.com. I propose we simplify the Platform enum to be just Youtube and URL.

@regiskuckaertz
Copy link

I see, that's smart!

Then how do you know which asset is for which platform? Is it that URL is a fallback case if there isn't anything more specific, and so currently all platforms except Youtube would use the URL asset?

I wonder if turning assets into a map<Platform, Asset> would make sense here.

Copy link

@sihil sihil left a comment

Choose a reason for hiding this comment

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

I think generally this seems like a sane approach. How will things be migrated?

FEATURE = 2,
NEWS = 3,
HOSTED = 4, // commercial content supplied by advertiser
PAIDFOR = 5
Copy link

Choose a reason for hiding this comment

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

Can you have a hosted documentary? Are these different things? Oh - it's not used anywhere anyway...

@jukecraft
Copy link

@akash1810 another old PR - time to close or salvage?

@regiskuckaertz
Copy link

Since we're adding an AudioAtom, I think it makes less and less sense to have a MediaAtom and would be super glad if we could get this through.

@akash1810
Copy link
Member Author

Since we're adding an AudioAtom, I think it makes less and less sense to have a MediaAtom and would be super glad if we could get this through.

Would love to chat what a CAPI migration would look like for this.

@regiskuckaertz
Copy link

@akash1810 @franziskas so you're aware, we'll be going ahead with this after we've done all the work to remove the mapping type in elasticsearch 😇

@akash1810
Copy link
Member Author

👋🏽 @guardian/content-platforms I just remembered this PR. Do you know how far Regis got with it? And/or shall we close it?

@annebyrne
Copy link

i don't know why but for some reason im on email thread for this 😢 ❤️ am i still on an old team by accident?

@marialivia16
Copy link

I got this as well, hello everyone! 😁

@akash1810
Copy link
Member Author

Hey @annebyrne @marialivia16 👋🏽. Totally surprised by that! I suspect you are/were repo watchers? In any case, hope you're both keeping well?

@annebyrne
Copy link

nah only watching for things i participated in but can't see myself in thread. just wanted to flag anyway in case i was on a github team somewhere by accident ☺️

@akash1810
Copy link
Member Author

akash1810 commented Jan 16, 2023

Closing, as unlikely to be worked on.

@akash1810 akash1810 closed this Jan 16, 2023
@akash1810 akash1810 deleted the aa-proposed-video-atom branch January 16, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants