Opened 12 years ago

Closed 12 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).

Change History (8)

comment:1 by Christopher Allan Webber, 12 years ago

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 by joar, 12 years ago

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

comment:3 by Christopher Allan Webber, 12 years ago

Milestone: 0.3.3

comment:4 by Christopher Allan Webber, 12 years ago

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 :)

in reply to:  4 ; comment:5 by joar, 12 years ago

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 12 years ago by joar (previous) (diff)

in reply to:  5 comment:6 by joar, 12 years ago

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 by Christopher Allan Webber, 12 years ago

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 by Christopher Allan Webber, 12 years ago

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.