Opened 8 years ago

Closed 8 years ago

#578 closed enhancement (fixed)

Do not transcode video if it does not make sense

Reported by: spaetz Owned by: joar
Priority: critical Milestone: 0.3.3
Component: programming Keywords: review
Cc: Parent Tickets:

Description

I uploaded a 320x200 pixel .webm file and it was transcoded to a 640x400 (or so) .webm file for previewing purposes, blowing up the file size by the factor of 3. This clearly does not make sense.

We should: 1) not transcode if we already have a smallish .webm file.
2) Provide an site option to disable transcoding and always just present the uploaded original file
3) Probably also disable transcoding for other common .ogg videos? (not sure on this one).

Subtickets

Change History (8)

comment:1 Changed 8 years ago by Christopher Allan Webber

Priority: majorcritical

Plenty of people have complained about this already. But also, the FSF has set up a MediaGoblin intstance and has said that the requirement to re-transcode video where it shouldn't be necessary is currently the most serious blocker to them making it live.

That makes this a very high priority. Bumping to critical.

comment:2 Changed 8 years ago by joar

Owner: set to Christopher Allan Webber
Status: newassigned
Type: defectenhancement

comment:3 Changed 8 years ago by Christopher Allan Webber

Milestone: 0.3.3

comment:4 Changed 8 years ago by Christopher Allan Webber

So, this looks good.

Three comments:

1) We should reflect in the <video> tag what the type is. Currently we do:

    <source src="{{ request.app.public_store.file_url(
      	   media.media_files['webm_640']) }}"
            type="video/webm; codecs=&quot;vp8, vorbis&quot;" />

What's the right way to resolve this? My suspicion is that we should do two things:

  • Add a media_entry.media_data.type() method that tries to supply the right info, and call it in the template (escaping &quot and etc appropriately)
  • Maybe actually at processing time supply that data and store it in the media data. (We could provide a migration for this that just sets everything currently to webm)

2) It looks like if someone set all the config options for "restrictions" to nothing, instead of always transcoding (what I would expect) it would never transcode?

3) I wonder if we shouldn't do the same thing for audio!

Let's talk tomorrow if you can :)

comment:5 in reply to:  4 ; Changed 8 years ago by joar

Replying to cwebber:

So, this looks good.

Three comments:

1) We should reflect in the <video> tag what the type is. Currently we do:

    <source src="{{ request.app.public_store.file_url(
      	   media.media_files['webm_640']) }}"
            type="video/webm; codecs=&quot;vp8, vorbis&quot;" />

What's the right way to resolve this? My suspicion is that we should do two things:

  • Add a media_entry.media_data.type() method that tries to supply the right info, and call it in the template (escaping &quot and etc appropriately)
  • Maybe actually at processing time supply that data and store it in the media data. (We could provide a migration for this that just sets everything currently to webm)

The codecs part may be omitted. The mimetype will be sent in the response for the source file request. The only case where it would be truly beneficial to provide a mimetype in the type argument would be if we had several source alternatives. We should however remove the type="video/webm ... part for videos that are not transcoded to WebM. Perhaps an 'is transcoded' flag somewhere?

The type attribute gives the type of the media resource, to help the user agent determine if it can play this media resource before fetching it. If specified, its value must be a valid MIME type. The codecs parameter, which certain MIME types define, might be necessary to specify exactly how the resource is encoded. [RFC4281]
-- http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#the-source-element

RFC4281 seems dated and does not contain neither 'vorbis' nor 'vp8', neither does the MP4reg register.

Replying to cwebber:

2) It looks like if someone set all the config options for "restrictions" to nothing, instead of always transcoding (what I would expect) it would never transcode?

It doesn't look like that to me. skip_transcode returns True for passes, and the tests are most if val in list(), it would quit really early if you had all of them set to [].

3) I wonder if we shouldn't do the same thing for audio!

That's a good idea, but that would have to be another ticket :)

Last edited 8 years ago by joar (previous) (diff)

comment:6 in reply to:  5 Changed 8 years ago by joar

The codecs part may be omitted. The mimetype will be sent in the response for the source file request. The only case where it would be truly beneficial to provide a mimetype in the type argument would be if we had several source alternatives. We should however remove the type="video/webm ... part for videos that are not transcoded to WebM. Perhaps an 'is transcoded' flag somewhere?

Actually, the type attribute could be dropped for all videos.

comment:7 Changed 8 years ago by Christopher Allan Webber

Keywords: review added
Owner: changed from Christopher Allan Webber to joar

The type bit turns out couldn't be dropped for all videos due to video.js stuff.

Anyway, I've uploaded a branch here:

https://gitorious.org/~cwebber/mediagoblin/cwebbers-mediagoblin/commits/joar-skip_transcoding

Joar, could you possibly review?

comment:8 Changed 8 years ago by Christopher Allan Webber

Resolution: fixed
Status: assignedclosed

I've gone ahead and merged this. Joar, if you could still look at it, that would be great, but I think between Elrond and I's reviewing I suspect it's safe.

Note: See TracTickets for help on using tickets.