WordPress.org

Make WordPress Core

Opened 14 months ago

Closed 13 months ago

Last modified 13 months ago

#23673 closed enhancement (fixed)

Add functions to generate metadata for video / audio

Reported by: wonderboymusic Owned by: markjaquith
Milestone: 3.6 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description

If you don't know the width and height of an uploaded video, you don't know the proper aspect ratio to frame it in a generic player environment.

As it stands now, uploaded videos have no metadata other than _wp_attached_file(), the file extension, and $post->post_mime_type. Let's investigate ways to get more data for videos in the media library upon upload, specifically dimensions and hopefully length, etc.

Attachments (6)

23673.diff (939.7 KB) - added by wonderboymusic 14 months ago.
ID3.zip (192.4 KB) - added by wonderboymusic 14 months ago.
unzip and drop in wp-includes/
23673.2.diff (3.6 KB) - added by wonderboymusic 14 months ago.
23673.3.diff (9.5 KB) - added by wonderboymusic 14 months ago.
23673.4.diff (7.7 KB) - added by wonderboymusic 14 months ago.
23673.5.diff (8.5 KB) - added by wonderboymusic 14 months ago.

Download all attachments as: .zip

Change History (21)

wonderboymusic14 months ago

comment:1 wonderboymusic14 months ago

  • Keywords has-patch added; needs-patch removed

attachment:23673.diff uses a subset of the getID3 library (GPL-licensed) to parse media files on metadata generation.

Introduces the 3rd-party getID3 library in wp-includes/ID3
Introduces wp_read_video_metadata( $file ) and wp_read_audio_metadata( $file )

Tested .mp3, .ogg, .wma, .m4a, .wav, .mp4, .m4v, .webm, .ogv, .wmv, .flv

Note: this patch does not require any other patch, and vice versa. The parsing gets us dimensions for video, which automatically show up here: http://cl.ly/image/1T3M2Q3A212Z

We also get a subset of the ID3 data for the media, a similar amount of data compared to images. Bitrate, Codec, Length, etc.

Last edited 14 months ago by wonderboymusic (previous) (diff)

wonderboymusic14 months ago

unzip and drop in wp-includes/

wonderboymusic14 months ago

comment:2 wonderboymusic14 months ago

attachment:23673.3.diff adds some dazzle, get ready for your mind to be blown:

  • Allows the Post Thumbnail metabox to show up for attachments that are audio or video, without explicitly supporting post-thumbnails in your theme or on the post type. Why? I have an MP3 with an embedded image, how do I associate them? Well, by just adding _thumbnail_id to post meta for the mp3, I have associated them just enough to be useful.
  • When generating metadata for audio or video: if your file contains an embedded image, I am now storing that binary chunk as an image file in the Media Library and adding _thumbnail_id to the media file's post meta. It's magic, and it's awesome.
  • The ID3 tags, when present, contain metadata like "album" "artist" "song" etc - we probably want to start displaying that data on the edit attachment screen so it can be changed / corrected. When the media file's metadata is stored, that data is stored along with it.

attachment:ID3.zip​ can be unzipped in wp-includes - I removed it from the patch because the .diff file was huge with it in there.

Subsequent changes to edit-form-advanced.php really require #23282 to be in, so let's get on that. This stuff is exciting because it is putting audio / video on par with images finally.

wonderboymusic14 months ago

comment:3 wonderboymusic14 months ago

  • Summary changed from Add functions to get metadata for videos to Add functions to generate metadata for video / audio

wonderboymusic14 months ago

comment:4 wonderboymusic14 months ago

Patch updated to use wp_upload_bits() and moves metadata code to wp-admin/includes/media.php

comment:5 nacin14 months ago

With regards to supporting featured images:

  • Theme support or 'attachment' or all should be required
  • Post type support should be required

Giving 'thumbnail' support to attachments is a bit weird, though. Instead, perhaps, we should implement something like

add_post_type_support( 'attachment:video', 'thumbnail' );
add_post_type_support( 'attachment:audio', 'thumbnail' );

wonderboymusic14 months ago

comment:6 wonderboymusic14 months ago

Patch updated to check for theme and post type support - themers are now blessed with being able to add these lines to their functions.php blob:

add_post_type_support( 'attachment:audio', 'thumbnail' );
add_post_type_support( 'attachment:video', 'thumbnail' );
add_theme_support( 'post-thumbnails', 'attachment:audio' );
add_theme_support( 'post-thumbnails', 'attachment:video' );

Laundry list of what is in this patch:

  • post type and theme support checks on the edit attachment screen
  • Introduce wp_read_video_metadata()
  • Introduce wp_read_audio_metadata()
  • Introduce getID3 library subset, ID3.zip needs to be unzipped in wp-includes/
  • Upload image from ID3 tags to Media Library if theme and attachment:audio|video support it
  • Introduce wp_add_id3_tag_data

comment:7 markjaquith13 months ago

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from new to closed

In 23766:

Add functions for generating metadata for video and audio, using the
ID3 library. Also allows themes/plugins to add thumbnail support
to these media types. Think stuff like album art, movie covers, and
video freeze-frames.

props wonderboymusic. fixes #23673

comment:8 follow-up: bpetty13 months ago

  • Cc bpetty added
  • Resolution fixed deleted
  • Status changed from closed to reopened

All files included in this 3rd party library mention a readme.txt, but it's been stripped out of what is included with WP, and none of the files mention what this library is licensed under (I couldn't find it without hitting up the library's website).

It is also interesting to note that this library is actually dual-licensed with GPL and a commercial license, thus it's really not quite compatible with the conditions WP is released under. For example, I could sell WordPress to someone (and what web designer doesn't sell a copy of WP to their clients technically?) without requiring anything beyond complying with the terms of the GPL, but with this library included, I would technically be required to purchase a commercial license from getID3 to do so.

It should be clear to anyone looking at this code (as distributed from WP) what it's license and conditions are, and it should probably include both licenses with the library here.

Also, I know this has been here for review for a while, but was there really not any way this could have been built in a more modular way so we're not including 20k lines of new PHP code just for this library (an additional 940KB for every WP install)? For reference, that's about 6k more than every WP_* class currently included in WP, 4k more than SimplePie, and accounts for 15% of everything in wp-includes.

This might only be used for maybe 10% to 15% of all installations, and even then, most users are only going to make use of the MP3 handler. They've have had plenty of ways of embedding audio and video into their site without needing any of this for a long time. I'm not saying it isn't useful, just that it's a lot to include in WP by default for something so far from the 80/20 rule.

Last edited 13 months ago by bpetty (previous) (diff)

comment:9 in reply to: ↑ 8 bpetty13 months ago

Replying to bpetty:

It is also interesting to note that this library is actually dual-licensed with GPL and a commercial license, thus it's really not quite compatible with the conditions WP is released under. For example, I could sell WordPress to someone (and what web designer doesn't sell a copy of WP to their clients technically?) without requiring anything beyond complying with the terms of the GPL, but with this library included, I would technically be required to purchase a commercial license from getID3 to do so.

Nevermind, getID3.org clarifies this a little more in their license FAQ. It's fine as long as WordPress source itself is there (and it should always be).

comment:10 wonderboymusic13 months ago

I hear you, dawg - that's why I brought it up in IRC a few weeks ago. If another ID3 parser exists that does this stuff and is mucho smaller, let's use it! However, I am only one man and this was the best library I found for the job - there was nothing else that was comparable.

Because there are so many formats of audio and video to support, and WordPress needs to be a facade for the parsing of metadata for all of these formats, this code isn't practical to bring in house. Any substitute would have to support ID3v1, ID3v2, APE, Vorbis Comment, RIFF, ASF parsing, etc etc - and by "parse" I mean "do byte stream analysis"

I am going to suggest we mark this fixed, ready to be reopened when someone comes marching in with an alternate solution.

comment:11 helen13 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Agree, going to leave this as fixed unless/until something actionable comes up. Mark did audit the library, and wonderboymusic is a person I trust when it comes to audio/video on the web, given the whole eMusic thing.

comment:12 bpetty13 months ago

I actually re-opened the ticket so license info could be added back in, as I'm sure the author intended to be distributed with the library, not because I necessarily felt that something needed to removed immediately.

Although I still don't see any reason we necessarily need to distribute WP with this builtin by default. WP can still support native audio/video without this library, and if someone wants this info to be parsed and available with their uploads, it could still be an optional plugin.

comment:13 bpetty13 months ago

By the way, I'm also reminded of #13307 in regards to this.

Are we still unzipping updates in-memory? (@dd32 and/or @ryan)

comment:14 follow-up: markjaquith13 months ago

I actually re-opened the ticket so license info could be added back in

Can you open a ticket for that?

comment:15 in reply to: ↑ 14 bpetty13 months ago

Replying to markjaquith:

I actually re-opened the ticket so license info could be added back in

Can you open a ticket for that?

Added: #23842

Note: See TracTickets for help on using tickets.