Page MenuHomePhabricator

misleading error about thumbnails for OGG audio files
Closed, ResolvedPublic

Description

The UploadWizard consistently offers the message "the upload succeeded but the server could not get a preview thumbnail" when uploading Ogg audio files. Screenshot:

phab_thumbnail_broken_for_ogg.png (259×769 px, 23 KB)

This seems pointless. Either recognize that the file is Ogg audio and provide a stock thumbnail or provide no thumbnail. Either way, there's no point in alarming the user that something may have gone wrong, when nothing in fact has.

Event Timeline

Restricted Application added subscribers: Zppix, Steinsplitter, Aklapper. · View Herald Transcript
matmarex subscribed.

That error message is actually a lie, the server does return a preview thumbnail (MediaWiki has a set of filetype icons to use for non-thumbnailable files, e.g. https://commons.wikimedia.org/w/resources/assets/file-type-icons/fileicon-ogg.png), but some logic is faulty and we're not using it.

Looks like we're incorrectly treating all .ogg files as video.

Actually, even though we do, that is not the problem. Client-side thumbnailing of audio fails, we the try the API, and it looks like the API response is wrong. For example:

https://commons.wikimedia.org/w/api.php?action=query&format=json&prop=stashimageinfo&siifilekey=1435cdq4h5nc%2Ezc4798%2E239936%2Eogx&siiprop=url&siiurlwidth=100&siiurlheight=100

{
  "batchcomplete": "",
  "query": {
    "stashimageinfo": [
      {
        "thumburl": "https://commons.wikimedia.org/w/resources/assets/file-type-icons/fileicon-ogg.png",
        "thumbwidth": 100,
        "thumbheight": 0,
        "url": "https://commons.wikimedia.org/wiki/Special:UploadStash/file/1435cdq4h5nc.zc4798.239936.ogx",
        "descriptionurl": "https://commons.wikimedia.org/wiki/Special:UploadStash/file/1435cdq4h5nc.zc4798.239936.ogx"
      }
    ]
  }
}

The "thumbheight": 0 confuses UploadWizard into thinking this is not a valid thumbnail. This problem does not occur locally for me – I get "thumbwidth": 120, "thumbheight": 120.

Thanks for quickly looking into it, @matmarex :)

Anomie subscribed.

The API is just returning what it's given. In this case what it's given is a TimedMediaTransformOutput that returns 100 for getWidth() and false for getHeight() (despite MediaTransformOutput::getHeight() being declared @return int).

This was created by TimedMediaHandler::doTransform(), which doesn't seem to handle the case of File::getWidth() and File::getHeight() returning false very well.

Thanks @Anomie, that was very helpful. (I should start adding the MediaWiki-Action-API tag to random tasks just to have you debug them. ;) )

Change 294121 had a related patch set uploaded (by Bartosz Dziewoński):
TimedMediaHandler: Don't try to calculate width and height for audio files

https://gerrit.wikimedia.org/r/294121

Except if it's obviously not related, I'll just untag it with a terse
response instead of debugging. ;)

This is a side effect of some old fundamental assumptions in the Media layers, that are no longer really true:

  1. Not everything is thumbnail-able with an image. This is really a html blob, but we have no provisions for that in the API layers. Therefor we 'stub' with a hardcoded image. It's a leftover from the old Ogg player from before TMH really.
  2. Not every thumbnail has both natural width and height. An audio clip has user defined width, and a hardcoded height, but no natural height or width at all. The problem here is.. the height of the hardcoded thumbnail is not the same as the height of the generated thumbnail player. We could hardcode the 'thumbnail' height in this case, but... I don't really want to, since that makes us divert even further from the fact that there is no thumbnail url for this....

My idea was to remove the hardcode audio thumbnail url altogether. It's also why I'm removing it as a 'poster' on audio clips recently. See T135603: Remove output of poster attribute for <audio> elements. Removing the thumbnail generation altogether would depend on that.

@matmarex I suspect your patch would make all audio players 120px wide, but I can't test right now...

I put some work into some audio element parsertests recently, that might show such a HTML output change in Jenkins:
https://gerrit.wikimedia.org/r/#/c/289436

@TheDJ I don't think it does that, since TimedMediaTransformOutput ignores the passed width/height for audio files. I couldn't get TimedMediaHandler to entirely work locally, though (just about enough to test those API responses), so I can't really say for sure…

matmarex triaged this task as Medium priority.Jun 14 2016, 7:22 PM

Change 294121 merged by jenkins-bot:
TimedMediaHandler: Don't try to calculate width and height for audio files

https://gerrit.wikimedia.org/r/294121

@matmarex if I understand it correctly, this patch overrides any explicit size set in audio files and forces it to 120px. Thus, the following code doesn't work anymore:

[[File:Example.ogg|15px|noicon]]

(I'll obviate the noicon part) This would output just the play/stop button and omit the rest of the player's layout. Such a shrinked version of the player had a purpose on eswiktionary, now it's shown in full length.

@matmarex if I understand it correctly, this patch overrides any explicit size set in audio files and forces it to 120px. Thus, the following code doesn't work anymore:

[[File:Example.ogg|15px|noicon]]

(I'll obviate the noicon part) This would output just the play/stop button and omit the rest of the player's layout. Such a shrinked version of the player had a purpose on eswiktionary, now it's shown in full length.

Hi Peter,

Can you link to an example on es.wikt?

we use the reduced version (15px) in https://es.wiktionary.org/wiki/Plantilla:aud (a subtemplate of
https://es.wiktionary.org/wiki/Plantilla:pron-graf)

here an example on how it looks now (but it shouldn't): https://es.wiktionary.org/wiki/rota#Latín

F. I thought I checked this.. Must have forgotten about object/parser caching, or the 404 thumbnailing behaves different than the immediate one.....

We should revert.

Reopening this, as I'm reverting the patch due to unintended side effects, as reported by users.

Change 298872 had a related patch set uploaded (by TheDJ):
Revert "TimedMediaHandler: Don't try to calculate width and height for audio files"

https://gerrit.wikimedia.org/r/298872

I'm very sorry for my late response, I was dealing with other things and this fell to the second page of my TODO list, so to speak. It looks like I indeed messed up all audio players by setting them to 120px width… If this is causing issues, you should have complained harder (best way to get things reverted is to file another bug and mark it as blocking next WMF deployment).

Now that I have looked at it again, I think there's an easy fix that doesn't require a revert? See the patch below. @TheDJ, can you review it? I'd like to have either it or your revert deployed soon.

Change 299616 had a related patch set uploaded (by Bartosz Dziewoński):
Unbreak audio thumbnail width

https://gerrit.wikimedia.org/r/299616

Wikitext to reproduce the problem:

[[File:Example.ogg|500px|thumb]]
[[File:Example.ogg|thumb]]
[[File:Example.ogg|120px|thumb]]
[[File:Example.ogg|15px|thumb]]

[[File:Example.ogg|500px]]
[[File:Example.ogg]]
[[File:Example.ogg|120px]]
[[File:Example.ogg|15px]]

The width should be respected in all cases (using the thumbnail size from user's preferences if none is provided), it result in a 120px player in all cases now.

Change 299616 merged by jenkins-bot:
Unbreak audio thumbnail width

https://gerrit.wikimedia.org/r/299616

Change 298872 abandoned by Bartosz Dziewoński:
Revert "TimedMediaHandler: Don't try to calculate width and height for audio files"

Reason:
I submitted a fix instead: https://gerrit.wikimedia.org/r/299616. Hopefully this is no longer needed.

https://gerrit.wikimedia.org/r/298872

Issue with thumbnails should get fixed with this week's WMF deployment, between Tuesday and Thursday on all wikis per https://www.mediawiki.org/wiki/MediaWiki_1.28/Roadmap. Sorry about that again!