#24134 closed defect (bug) (fixed)
Video Shortcode ignores sizes and aspect ratio
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.6 | Priority: | normal |
Severity: | normal | Version: | 3.6 |
Component: | Media | Keywords: | has-patch |
Focuses: | Cc: |
Description
I think this may be related to #23955
Uploaded video and resized it:
[video width="300" height="400" mp4="http://elfshot.org/wp-content/uploads/sites/3/2013/04/IMG_0328.mp4"][/video]
Looks fine in the post editor, it's got the right aspects yay.
Then on the live post, the placeholder HTML5 shows sideways (landscape) but when I click play, it shows as full sized to the content.
http://test.ipstenu.org/video-test.2013/ has the example on TwentyThirteen but it happens on all videos on all themes that I've tested. When I viewed source the video tag has the size of the content area, as do media elements.
At revision 24036.
Attachments (4)
Change History (34)
#2
@
12 years ago
- Cc wonderboymusic added
- Keywords has-patch added; needs-patch removed
It looks like MediaElement.js is reading the max-width
CSS property and ignoring the specified width and height if it's 100%, which is the case in both Twenty Thirteen and Twenty Twelve. It's what makes both themes responsive.
I think MediaElement.js is making the wrong assumption here, but it's what makes the video responsive.
A workaround would be to wrap the video
element with a div, so that even if the video's width is set to 100%, it is still constrained by the parent block, and still responsive because both elements would have a 100% max-width. This, however, does not work for the video height. See 24134.diff.
#3
@
12 years ago
Shouldn't it grab the max width from the video Shortcodes, when applied?
Also max width going larger than the actual video dimensions is sort of poor UI. It makes the videos harder to see, which is terribly evident in vertical vids. That video was saved as 600x800
#4
@
12 years ago
Tested - This patch fixes half the problem (YAY!) and resizes as defined.
It does not fix the problem where the video placeholder loads landscape no matter what. The placeholder issue is way less of a concern than the ignoring of video sizes, though. This needs to be in core to at least get it working as designed.
#6
@
12 years ago
placeholder in landscape.... this problem is kind of a nightmare right now - the getID3 library returns dimensions as resolution_x
and resolution_y
but doesn't say anything about orientation, it appears that "width" is always the longer side. I need to do some more research on this, the code that deserializes the byte stream into ID3 is unbelievably complex.
#7
@
12 years ago
From what I am reading, video is always stored wider (portrait is stored as landscape, and might be played back portrait if the app knows what it is doing) - gotta find out if there is transform info stored somewhere in the ID3 stream
#8
@
12 years ago
Commands like ffmpeg -i IMG_0463.mov
will return metadata, including a rotate: 90
declaration, which is what we need.
#10
follow-up:
↓ 11
@
12 years ago
I don't see any solution right now - my only hunch is that reading the Apple-specific metadata for an Apple-generated file would allow us to purposely do a 90 degree CSS3 transform on a portrait video to make it appear correct. That's the best case scenario which I have no code to support. Seems like this data would be in the ID3 library if it was possible to extract reliably.
The main issue is probably the endless number of vendors' metadata to read to know what the rotate
of the phone was when the video was recorded. Is there any other scenario where a video would be in portrait orientation, other than when recorded wrong with a phone?
Maybe I've been staring at the code too long and am going insane, am I missing something?
#11
in reply to:
↑ 10
@
12 years ago
Replying to wonderboymusic:
The main issue is probably the endless number of vendors' metadata to read to know what the
rotate
of the phone was when the video was recorded. Is there any other scenario where a video would be in portrait orientation, other than when recorded wrong with a phone?
Given ID3 is an absurd amount of code, I'd hope it would handle this for us. Since it doesn't, seems like we shouldn't even attempt (considering they've thought of everything else).
#12
@
12 years ago
Maybe I'm missing something here, but I had the impression the issue is partly a) trying to serve responsive videos and partly b) the video loading with the correct dimensions only to have Mediaelement override with inline styles. Followed by, of course, clicking play and the video loading at max-width which in some cases ends with a vertical video getting scaled up to fit that width.
So I'm not sure what all of this about positioning and 90 degree transforms has to do with it.
Mediaelement is:
1) overriding the dimensions to fit max-width of the container
2) Loading the start screen at a dimensional ratio consistent with max-width, e.g. 604px wide in Twenty Thirteen with a landscape-equivalent height:width ratio regardless of what the actual dimensions are.
#13
follow-up:
↓ 14
@
12 years ago
I never got a portrait mode video to play in portrait in Chrome - tried 8 different videos with setting changes in iMovie.
#14
in reply to:
↑ 13
@
12 years ago
Replying to wonderboymusic:
I never got a portrait mode video to play in portrait in Chrome - tried 8 different videos with setting changes in iMovie.
This is in FF22: http://screencast.com/t/OlzLhUQypSfs
See the jump on refresh?
#15
follow-up:
↓ 16
@
12 years ago
yeah - the kovshenin patch seems to help that, but I think the bigger issue is that portrait videos don't reliably play as portrait.
Main reason: we actually have no idea when the video is portrait, unless the user changes the dimensions by hand. Even then, the video probably will be laying on its side.
#16
in reply to:
↑ 15
@
12 years ago
Replying to wonderboymusic:
yeah - the kovshenin patch seems to help that, but I think the bigger issue is that portrait videos don't reliably play as portrait.
Main reason: we actually have no idea when the video is portrait, unless the user changes the dimensions by hand. Even then, the video probably will be laying on its side.
Yeah, and then we have other problem it scaling up to fit max-width of the container: http://screencast.com/t/nQTExszxvNG2
#17
@
12 years ago
Another consideration is how videos scales responsively (phones, tablets, etc) which is an entire new can of worms.
Possibly a fix for the entire video issue might be using http://fitvidsjs.com to handle responsive scaling.
I'm probably out of line about this seems like the issue of responsive video handling might best be figured into the mix.
#18
@
12 years ago
@wonderboymusic:
I just realized that we have no difficulty loading videos of all shapes and sizes in the attachment editing UI. Videos are loaded, displayed, and played with the proper dimensions no problem. This is the editing screen for the video I commented on above: http://cl.ly/image/2L173S3A141O
So if we're not trying to enforce responsiveness in the attachment UI, I'm not sure why we need to do it on the front-end. Let core handle outputting the content as intended and themes can handle the rest with fitVids or whatever.
#19
@
12 years ago
What should we do here for 3.6? Anything? Is this a me.js issue? Is this no-one's issue? What are the actual issues? I seem to notice two — portait videos and 100% max-widths.
#20
@
12 years ago
Ignoring Aspect Ratio is clearly a PITA, and a messier problem.
But the shortcode ignoring sizes is, to me at least, the biggest issue, since it's outright breaking how shortcodes are expected to work. If we can fix that, then the aspect ratio can be bumped I think since it'll require some reworking.
Or at least separate the problems into "What can we fix for 3.6 and what can we not fix in time..."
#21
@
12 years ago
So the issue is just how me.js responds when a theme tries to make it responsive width?
#23
@
12 years ago
Kovshenin's div wrapper works (tested in 2013) to fix that issue, at least. Still applies.
#24
@
12 years ago
24134.3.diff Fixes this issue AND #24798 by defaulting preload
to metadata
for videos.
Additionally:
$attr
passed through the filter is an actual key/value array instead of an array ofattr="val"
strings.- Respects
preload
,autoplay
,loop
, through the shortcode (both audio and video). - Includes Kovshenin's div wrapper to fix the responsiveness bug.
#25
@
12 years ago
- Owner set to markjaquith
- Resolution set to fixed
- Status changed from new to closed
In 24789:
#26
@
12 years ago
Hi
First post on WP Trac, sorry if I'm doing things wrong...
In changeset [24789], I found it was very annoying having that div wrapped without a class or anything to refer using CSS...
It makes for instance very difficult to center the video, as the wrapped div has to be targeted.
I thing adding a class would greatly help.
#27
follow-up:
↓ 29
@
12 years ago
I've just discovered that my width and height attributes from my shortcode are overwritten in the wp_video_shortcode function.
I've commented these lines out and its working again.
elseif ( ! is_admin() && $w > $defaults_atts['width'] ) $w = $defaults_atts['width'];
This [video src="http://xxx/wp-content/uploads/2013/08/ifa-cs.edit_.fx_.012_finish.flv" width="815" height="459"] will result in a video width 640 width and 360 height. Anyone got the same problem?
#28
@
12 years ago
Replying to AlphaK:
In changeset [24789], I found it was very annoying having that div wrapped without a class or anything to refer using CSS...
Replying to Horttcore:
I've just discovered that my width and height attributes from my shortcode are overwritten in the wp_video_shortcode function.
This ticket was closed on a completed milestone. Please create new tickets for new issues.
Reproduced this with Twenty Thirteen and Twenty Twelve, but not Twenty Eleven and Twenty Ten.