Opened 5 years ago

Last modified 3 years ago

#1004 accepted defect

Videoplayer has wrong height for videos with width>640

Reported by: Jaakko Luttinen Owned by:
Priority: major Milestone:
Component: programming Keywords: video
Cc: Parent Tickets:

Description

It is possible to skip transcoding videos that are larger than 640 pixels in some dimension. For instance, this way a user can upload 1280x720 webm videos without transcoding them to smaller. However, there is a bug in the player for these videos.

If a video is wider than 640 pixels, the width of the videoplayer is scaled to 640 pixels but the height of the videoplayer is the same as of the video. Thus, for a 1280x720 video, the player has size 640x720, leaving black bars on top and bottom of the video. Fullscreen obviously works fine.

A sketch of a solution:
HTML video player is scaled appropriately if only width or height is given. Thus, give the width as min(width,640) and let the height be chosen automatically.

Modify the video tag in the template mediagoblin/templates/mediagoblin/media_displays/video.html:

<video controls
          {% if global_config['plugins']['mediagoblin.media_types.video']['auto_play'] %}autoplay{% endif %}
          {% if media.media_data.width > 640 %}width=640{% else %}width={{ media.media_data.width }}{% endif %}
          preload="auto" class="video-js vjs-default-skin">

A few things to consider:

  • I hardcoded 640, but it should be retrieved somehow.
  • I removed data-setup because it breaks this fix and I think it's not valid HTML

Subtickets

Change History (4)

comment:1 Changed 5 years ago by Jaakko Luttinen

Last edited 5 years ago by Jaakko Luttinen (previous) (diff)

comment:2 Changed 5 years ago by Jaakko Luttinen

Status: newreview

comment:3 Changed 4 years ago by Loic Dachary

Status: reviewaccepted

Could you please attach the patch to this ticket ? gitorious is defunct.

comment:4 Changed 3 years ago by phihos

Since the code has changed a little bit and setting the width alone causes black bars at the side, I throw in my five cents:

<video controls
         {% if global_config['plugins']['mediagoblin.media_types.video']['auto_play'] %}autoplay{% endif %}
         preload="auto" class="video-js vjs-default-skin"
         data-setup='{ {% if media.media_data.width > 640 %}"width": 640, "height": 360{% else %}"width": {{ media.media_data.width }}, "height": {{ media.media_data.height }}{% endif %} }'>

Obviously I hardcoded the aspect ratio to 16:9. There might be a better way to do this.
Could you consider fixing this for 1.0? This seems easy to fix and is very annoying.

Anyway thank you very much for this awesome project :-)

Note: See TracTickets for help on using tickets.