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

Video support #1165

Closed
wants to merge 26 commits into from
Closed

Conversation

theSuess
Copy link
Contributor

@theSuess theSuess commented Nov 28, 2022

Implements #1089

Pretty much what it says in the title 😁

This add support for the video/mp4 mime type with either ffmpeg based thumbnail generation or a go-native placeholder solution.

This is still a draft and further cleanup/optimizations might be required

Copy link
Contributor

@f0x52 f0x52 left a comment

Choose a reason for hiding this comment

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

web stuff looks alright

web/source/css/status.css Show resolved Hide resolved
tsmethurst and others added 9 commits November 29, 2022 09:24
* start refactoring some of the search + deref logic

* add tests for search api

* rename GetRemoteAccount + GetRemoteStatus

* make search function a bit simpler + clearer

* fix little fucky wucky uwu owo i'm just a little guy

* update faulty switch statements

* update test to use storage struct

* redo switches for clarity

* reduce repeated logic in search tests

* fastfail getstatus by uri

* debug log + trace log better

* add implementation note

* return early if no result for namestring search

* return + check on dereferencing error types

* errors hah what errors

* remove unneeded error type alias, add custom error text during stringification itself

* fix a woops recursion 🙈

Signed-off-by: kim <grufwub@gmail.com>
Co-authored-by: kim <grufwub@gmail.com>
…sbusiness#1176)

Signed-off-by: kim <grufwub@gmail.com>

Signed-off-by: kim <grufwub@gmail.com>
…superseriousbusiness#1177)

* only return error for emoji fetch if NOT errnoentries

Signed-off-by: kim <grufwub@gmail.com>

* reformat gts->api model slice conversion to standard error behaviours and reduce code reuse

Signed-off-by: kim <grufwub@gmail.com>

Signed-off-by: kim <grufwub@gmail.com>
…ing them (superseriousbusiness#1188)

* add predictable instance account to tests, use it in emoji urls + paths

* use static image url to select emojis when pruning orphaned
…ccount by domain+username (superseriousbusiness#1190)

* don't lowercase account username when doing a select

* test getting remote user with uppercase username
…rseriousbusiness#1183)

* Enable the 'admonitions' Markdown extension for Mkdocs.

The admonitions extension to Python-Markdown allows you to include
rST-style "admonitions" to Markdown documents, for instance,

    !!! note
        Here's an important note to keep in mind!

In general, the current documentation uses bold text to try to achieve
the same effect, which is a bit harder to notice and makes it difficult
to differentiate between "here's something useful to know" versus "here
there be dragons".

* Add AppArmor profile and documentation for LSM-related sandboxing

This commit adds an AppArmor profile for gotosocial in
examples/apparmor/gotosocial. This will (hopefully) serve as a helpful
security mitigation for people are planning on deploying GTS on a
Debian-family Linux distribution.

I've also updates the documentation to include some information about
deploying GTS with either AppArmor or SELinux (moving the documentation
for the former out of the "binary installation guide" docs).
…business#1179)

* ap: add support for PKCS1 "RSA PUBLIC KEY" pem block type

Signed-off-by: Sigrid Solveig Haflínudóttir <sigrid@ftrv.se>

* ap: report no PEM data or unknown pem block type

Signed-off-by: Sigrid Solveig Haflínudóttir <sigrid@ftrv.se>

Signed-off-by: Sigrid Solveig Haflínudóttir <sigrid@ftrv.se>
Copy link
Contributor

@tsmethurst tsmethurst left a comment

Choose a reason for hiding this comment

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

Looking good :) Couple comments about the implementation stuff.

Re: calling ffmpeg/ffprobe as a command, I know @NyaaaWhatsUpDoc still had some doubts about this. Maybe we could talk about that here?

if err != nil {
return nil, err
}
case <-time.After(time.Second):
Copy link
Contributor

Choose a reason for hiding this comment

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

could this not take longer than 1s for large videos or slow machines? perhaps it's worth increasing this

edit: wait, i guess this just reads info from the header rather than looking through the whole file right? :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should make the timeouts configurable as the performance will vary across hardware specifications


func decodeVideoFFMPEG(r io.Reader, contentType string) (*mediaMeta, error) {
prog := "ffprobe"
args := []string{
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it's worth commenting these args / linking to docs for ffprobe for them if possible, to make this call easier to understand

}, nil
}
func decodeVideoNative(r io.Reader, contentType string) (*mediaMeta, error) {
data, err := ioutil.ReadAll(r)
Copy link
Contributor

Choose a reason for hiding this comment

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

reading the whole thing into memory seems expensive, since videos can be quite large... is there a way to avoid doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, sadly not possible since the mp4 parser needs to be able to seek the stream at will. To avoid excessive memory usage one could write the stream into a file first and then read from disk

}
func extractFromVideoFFMPEG(r io.Reader) (image.Image, error) {
prog := "ffmpeg"
args := []string{
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as above re: args

func extractFromVideoNative(r io.Reader) (image.Image, error) {
// As of writing this, no go native way exists to extract a thumbnail from a
// video so we fall back by just returning a static file
file, err := os.Open(path.Join(config.GetWebAssetBaseDir(), "logo.png"))
Copy link
Contributor

Choose a reason for hiding this comment

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

there's probably a better way of doing this than decoding the same logo every time :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should be possible to load it during package initialization, but not sure how the config module handles this. Otherwise I'll have to wrap the function onto a struct and initialize it somewhere else

tsmethurst and others added 14 commits December 1, 2022 16:06
… username/domain (superseriousbusiness#1191)

* [bugfix] Case-insensitive account selection

* don't lowercase cache key
…s#1195)

In the previous changes that expanded the IPv4 and IPv6 deny lists based
on the IANA registries we inadvertently added a number of duplicates.
This is unnecessary as they're already caught by larger prefixes and
means there's less entries to scan.

This change removes all prefixes that are subnets of other prefixes.
Implements superseriousbusiness#864 and should speed up s3 based installations by a lot.

With more static urls, we can then also implement superseriousbusiness#1026 for even
better performance when used in conjunction with CDNs
…ess#1207)

This fixes a couple of cases where due to int being platform dependent a
value could get truncated if running on 32bits.
… config (superseriousbusiness#1206)

* remove filesystem logging directives from example systemd unit config

* [docs] Update docs to reflect new systemd config

Co-authored-by: tsmethurst <tobi.smethurst@protonmail.com>
* media: add webp support

Signed-off-by: Sigrid Solveig Haflínudóttir <sigrid@ftrv.se>

* bump exif-terminator to v0.5.0

Signed-off-by: Sigrid Solveig Haflínudóttir <sigrid@ftrv.se>

Signed-off-by: Sigrid Solveig Haflínudóttir <sigrid@ftrv.se>
* [feature] overhaul the oidc system

this allows for more flexible username handling and prevents account
takeover using old email addresses

* [feature] add migration path for old OIDC users

* [feature] nicer error reporting for users

* [docs] document the new OIDC flow

* [fix] return early on oidc error

* [docs]: add comments on the finalization logic
@tsmethurst
Copy link
Contributor

I'm gonna close this now since we pulled in video support (based on the excellent work done in this PR) in #1274. Thank you Dominik!

@tsmethurst tsmethurst closed this Dec 17, 2022
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.

None yet

7 participants