Audio uploads feature

[DJSundog]

This adds the ability for users to upload audio files which are then transcoded to h264/aac/mp4 videos

I tried to keep this as close to the spirit of the video handling code as possible

[hannahwhy]

Taking a look now (read: deploying it on my instance, getting a feel for its function, and then examining the code).

[hannahwhy]

Very pedantic detail: indentation in the Mastodon codebase is 2 spaces. Deleting one column of spaces here should do the trick.

[DJSundog]

heh, whoops! I'll go get that :)

[hannahwhy]

I tried uploading a 4-minute song to my Mastodon instance (the VPS it runs on has so far been fast enough to handle all other Mastodon tasks) and I received a gateway timeout. Investigation on the server revealed that ffmpeg is still running in the background. Evidently something in the transcode step is quite slow.

Even on my desktop (a Xeon E5520) the ffmpeg command proceeds only at ~1.6x real time, which for a 4-minute song will take much longer than 60 seconds to complete. (I chose 60 seconds because that's a popular timeout value; for example, the Heroku router will terminate requests that don't receive a response within 60 seconds.)

Removing the filter chain and video stream mapping does speed things up substantially (on my desktop: 1.6x -> 14x), but it also leaves you without a video stream. I think browsers can handle this -- I know Firefox will handle e.g. webms without video -- but I have not tried it yet.

Maybe encoding embedded cover art would be a good middle ground?

(Alternatively, is this feature not meant for 4-minute songs?)

[hannahwhy]

(I suppose The Real Answer is to make transcoding asynchronous, but that seems like a bigger job than "first iteration of audio uploads")

[MightyPork]

maybe if we gave up the fancy waveforms, it would go faster?

[hannahwhy]

It definitely goes faster without showwaves, but then you're left without a video stream. I'm checking to see if that works.

[DJSundog]

yeah, my original iteration was simply forcing mp3 into the video tag and it worked but felt ugly.

since Garg's original argument against this functionality was partially related to music piracy I wasn't worrying much about the failure of long content to work in a timely fashion if at all tbh. I'm sure there's an ffmpeg incantation that would grab the waveform as a png and use it as a static frame looped that might be lighter than showwaves without being completely black video.

[MightyPork]

That's a strange argument, it's quite trivial to rip the audio stream from an mp4...
i guess it would be ok if it simple rejected files over, say, 1 minute?

[DJSundog]

hm, could probably do a metadata check prior to the transcode much like the one that currently checks to see if a video has an audio track and get the duration there and raise an exception - I'll take a looksie

[hannahwhy]

paperclip-av-transcoder provides ways to limit attachments by size, but I don't think there's a built-in duration check. You could probably write one by invoking ffprobe, e.g.

ffprobe -of json -show_streams FILE

and then parsing the output (using jq syntax: .streams[] | .duration).

Another approach that doesn't quite offer the best UI feedback but does limit system load is to pass the -t option to ffmpeg, e.g. -t 10 will stop encoding after 10 seconds.

[DJSundog]

So, I am getting the duration and raising an error, but it comes through on the front end as a 500 error about thumbnail generation:

 module Paperclip
   class AudioTranscoder < Paperclip::Processor
     def make
       meta = ::Av.cli.identify(@file.path)
       # {:length=>"0:00:02.14", :duration=>2.14, :audio_encode=>"mp3", :audio_bitrate=>"44100 Hz", :audio_channels=>"mo no"}
       if meta[:duration] > 60.0
         raise Paperclip::Error, "Audio uploads must be less than 60 seconds in length."
       end

Anyone have a better error offhand to raise that will bubble better?

[hannahwhy]

In this case I think it'd be okay to make your own exception class and rescue it in Api::V1::MediaController, e.g.

module PaperClip
  class AudioTranscoder < Paperclip::Processor
    class TooLongError < StandardError
    end

    def make
      # ...
      raise TooLongError, 'Audio uploads must be less than 60 seconds in length'
    end
    # ...
  end
end

class Api::V1::MediaController < Api::BaseController
  def create
    # ...
  rescue Paperclip::AudioTranscoder::TooLongError => e
    render json: { error: e.message }
    # ...
  end
end

[DJSundog]

opps just saw your comment after I committed something using Mastodon::ValidationError that shows up as a 422 toast/flash message/thingie in the web UI with the text "Audio uploads must be less than 60 seconds in length" - will that do?

[hannahwhy]

Yeah, the 4xx code is appropriate and Mastodon::ValidationError has a handler in Api::BaseController, so it's probably fine for now -- definitely less code to write. If Mastodon::ValidationError ends up being redefined in the future it's not a big deal to change the exception class.

[DJSundog]

woot! thanks for the review and assistance!

[hannahwhy]

Sure, no problem. Let's see what @ekiru says :)

[beatrix-bitrot]

sooooooooooo i may be in a minority here but i'd prefer not to have any artificial limitation on the length of the clips. i think whatever you can get away with in the default file size limit (currently 8M for video?) should be acceptable.

I'm going to merge this as it is for now and we can at least have the feature to enjoy while we think about how to proceed.

Thanks for your work all <3

[rhaamo]

I think the reason to encode it as video is to be compatible with non-glitch-soc instances.

And for length it may be for file size limit like the video one ?

[hannahwhy]

The main reason for the length limit (at least in glitch-soc) is to avoid request timeouts in the upload handler; transcoding did not occur in the background when this PR was submitted. (It might now, though I don't think that's changed.)

The reason for transcoding the audio is that it seemed like the easiest way to get something working; the rest of the existing code deals with video playback. That said, audio provided as WebM/Opus/MP3 probably can be used as-is these days.

I think glitch-soc would be fine with additional refinement to remove both of these limits.

[DJSundog]

hi, sorry, took me a minute to figure out where this thread was happening, need more coffee

the transcode is indeed due to upstream not whitelisting the audio tag - I would very much prefer to just use the non-transcoded file in an audio tag, if this is going to get traction up there, but there have been Opinions about presentation, so this was a hacky workaround to bypass the Opinion.

I would be very happy to see this PR get superceded by a less hacky implementation using actual audio files.