Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#27573 closed enhancement (fixed)

Don't re-upload album covers that already exist

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


If I drag 5 files that have ID3 tags from the same album into the uploader...

5 images are uploaded

What should happen:
We store a hash of the bits with the attachment and check for it before re-uploading the image

Attachments (4)

27573.diff (2.5 KB) - added by wonderboymusic 8 years ago.
27573.2.diff (2.5 KB) - added by wonderboymusic 8 years ago.
27573.3.diff (3.0 KB) - added by wonderboymusic 8 years ago.
27573.4.diff (3.0 KB) - added by wonderboymusic 8 years ago.

Download all attachments as: .zip

Change History (15)

8 years ago

This ticket was mentioned in IRC in #wordpress-dev by wonderboymusic. View the logs.

8 years ago

#2 @GregLone
8 years ago

Erf, I was about to post almost the same thing -_-'

The difference is I used a filter before the thumbnail creation, so plugins can add a title to the attachment if they want (track title? album title?).

 * Filter the parameters for the attachment thumbnail creation.
 * @since 3.9.0
 * @param array $attachment An array of parameters to create the thumbnail.
 * @param array $metadata   Current attachment metadata.
 * @param array $uploaded   An array containing the thumbnail path and url.
$attachment = apply_filters( 'attachment_thumbnail_args', $attachment, $metadata, $uploaded );

$sub_attachment_id = wp_insert_attachment( $attachment, $uploaded['file'] );

Also, I used a post meta to store the hash, but post_content_filtered seems to be better. Less easy to get but we won't use it every day.

Last edited 8 years ago by GregLone (previous) (diff)

#3 @nacin
8 years ago

I understand why post_content_filtered was chosen, but I don't love it here. It also isn't indexed so it's not a whole lot better than, say, a meta query.

#4 @wonderboymusic
8 years ago

  • Keywords has-patch added

#5 @wonderboymusic
8 years ago

27573.2.diff implements this by using get_posts() and a key-only meta_query. Both patches' queries are sub-millisecond. This one is probably preferable because it uses more API and no direct SQL. This code will hardly ever run, so speed arguments may be moot.

#6 @GregLone
8 years ago

What about the filter?

#7 @GregLone
8 years ago

Owned by 70 seconds... XD

Thanks @wonderboymusic

#8 @wonderboymusic
8 years ago

27573.3.diff 80 seconds before your comment.

#9 @nacin
8 years ago

Rather than 'meta_query' => array( array( 'key' => $hash ) ) we can use 'meta_key' => $hash. That said, I don't really like the idea of an unprefixed md5 hash meta key floating around in the metadata table. This is a situation ripe for 'meta_key' => '_album_cover_hash', 'meta_value' => $hash. Speed is not super-essential here and it is hardly the first barebones meta key/value query we do in the admin.

#10 @wonderboymusic
8 years ago

27573.4.diff implements _cover_hash since this also applies to video

#11 @wonderboymusic
8 years ago

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

In 27863:

In wp_generate_attachment_metadata(), when an audio or video files contains upload-able image bits in its ID3 tags, only upload it if the image has not already been uploaded. Determine this by checking for a _cover_hash value in post meta that matches the md5 representation of the bits.

This prevents uploading an album of 10 songs and subsequently uploading 10 copies of the same album cover.

Props GregLone for the new filter/filter docs: 'attachment_thumbnail_args'.
Fixes #27573.

Note: See TracTickets for help on using tickets.