#35218 closed enhancement (fixed)
Parse the creation date out of uploaded videos
Reported by: | stevegrunwell | Owned by: | mikeschroder |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
When images are uploaded into the media library, wp_read_image_metadata()
will read the images' EXIF data to determine the date the image was taken, which is returned in "created_timestamp". Other media types (audio and video) do not provide this information, but it's still useful metadata (especially on media-heavy sites).
This information is available through the getID3 library, however where it lives varies based on the type of media uploaded. I propose we add a "created_timestamp" key to wp_read_video_metadata()
(and ideally wp_read_audio_metadata()
as well) which parses out a timestamp corresponding to when the media was created (as we do with images).
Attachments (14)
Change History (39)
#1
@
5 years ago
Note: the patch file attached references three video files, which are also attached. These short, 15 second clips in three different formats should live in tests/phpunit/data/videos/
This ticket was mentioned in Slack in #core-media by desrosj. View the logs.
3 years ago
#5
@
3 years ago
- Milestone changed from Awaiting Review to 4.9
- Owner set to joemcgill
- Status changed from new to assigned
Thanks @stevegrunwell for the work on this and sorry that it's taken so long to review. Let's take a look for 4.9.
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
3 years ago
#9
@
3 years ago
I dug into this a bit, and if I'm reading this correctly, it looks like we will need to do normalization on the values (or maybe grab from a different location/format for MOVs?).
The example MOV file, and one that I tested from a live photo, have creation dates that get stored as far in the future.
The one in the automated test, for example, is 3533823605
which looks to be 12/24/2081 @ 5:40pm (UTC).
#10
@
3 years ago
Looked into this and did some testing. Also noticed what @mikeschroder did. There is a creation_time_unix
index for Quicktime/MP4 files. Is there any reason not to use that value for the created time instead of the creation_time
one?
Incoming patch using that one instead, as well as updating the version in the inline documentation.
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
3 years ago
#12
@
3 years ago
Totally on board with using creation_time_unix
instead of creation_time
, as long as it's consistent. The ID3 documentation has a nice warning about the "subatoms" node:
This is an undocumentably-complex recursive array, typically containing a huge amount of seemingly disorganized data. Avoid this like the plague.
A creation date in the far future doesn't do anybody any favors :)
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
3 years ago
#15
@
3 years ago
- Owner changed from joemcgill to mikeschroder
- Status changed from assigned to reviewing
Reassigning to @mikeschroder for final decision, commit here.
#16
@
3 years ago
Per discussion in today's weekly component meeting, 35218.2.diff adds a filter to the function result. This will allow devs to hook in and parse the creation time for any file type that we do not decide to parse in Core.
#17
@
3 years ago
I updated the patch to include WebM. It also looks for Matroska creation times in a second place (an FFMPEG source file I tested with had it under [info]
instead of [comments]
). This patch also has binary data for some unit test files (all based off the existing small-video.mp4
.
It would be nice to get some OGG love in here too, but getID3
doesn't seem to fully support Theora yet.
@
3 years ago
These are the quickie files used in the unit tests, meant to be placed in tests/phpunit/data/uploads
This ticket was mentioned in Slack in #core-media by mike. View the logs.
3 years ago
@
3 years ago
Only create meta if there is a creation date and only rely on small* test media files.
#20
@
3 years ago
Chatted with @joemcgill about this tonight.
Three new patches, can commit one of them in the morning (or feel free to do it, @joemcgill if it needs to happen first thing, and you're working first).
I'm okay with all of these approaches, but prefer the last.
- 35218.6.diff is basically an update. Only create meta if there is a creation date and only rely on small* test media files.
- video_meta.diff places the new data (creation timestamp) into a
video_meta
array in meta, much likeimage_meta
on images.
- video_meta_expanded_filter.diff like the diff above, but also swaps the proposed filter for
wp_read_video_metadata
, which closely matcheswp_read_image_metadata
.
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
3 years ago
#22
@
3 years ago
I like the idea of putting the filter in wp_read_video_metadata()
as in video_meta_expanded_filter.diff, now with fresh eyes, I think putting creation time into a video_meta
array seems unnecessary. In reality, everything returned by wp_read_video_metadata()
is “video meta”, but we don’t want to change that storage at this point for back-compat reasons. Otherwise, this looks good for a commit today.
#23
@
3 years ago
Sure, @joemcgill, pick the one option I didn't leave a diff for!
I'm kidding, of course. Will work this up, thanks so much for the review!
Would appreciate any more eyes on the wp_read_video_metadata
filter, since it had to be just enough different from the image one to make it not directly interchangable.
First pass at wp_get_media_creation_timestamp()