Ascoware / get-iplayer-automator

Get iPlayer Automator allows you to let Apple's TV app and your Mac to become the hub for your British television experience.
https://ascoware.github.io/get-iplayer-automator/
GNU General Public License v3.0
147 stars 26 forks source link

Wrong album artwork for BBC R episodes #34

Closed elflife closed 7 years ago

elflife commented 7 years ago

What steps will reproduce the problem? Be specific, and provide as much detail as possible. Download any BBC Radio episodes that belongs to the same program but uses different album artwork for each episode. "Drama on 3 [http://www.bbc.co.uk/programmes/b006tnwj/episodes/player]", "Afternoon Drama", "The Reunion", etc. When complete, the album artwork won't be the one accompanying each episode, but the same one on the program's home page.

What is the expected output? What do you see instead? Expected: Each episode with its individual artwork. Get: The same one (the one on the program's home page) for all of them.

What version of GiA are you using? On what version of OS X? Version 1.9b6 (1833) on 10.12.4

Are you using a web proxy? DNS proxy? VPN? VPS? No, no, no and no.

What program are you attempting to download (full title, series and episode)? Provide only ONE program as an example. For example, "Afternoon Drama", the program "Mark Burgess - A King's Speech[http://www.bbc.co.uk/programmes/b00k3xld]". The album artwork should be this photo [https://ichef.bbci.co.uk/images/ic/640x360/p01gg1pg.jpg], but when complete in iTunes, it was this photo: (p01ynb8q.jpg)[http://www.bbc.co.uk/programmes/b006qrzz] 20170517-052547_screen

Please provide any additional information below. All the other tagging info were correct. Also, programs with one artwork/cover throughout the series are perfectly fine.

Vangelis66 commented 7 years ago

This is again fallout from the move from get_iplayer 2.94 (used in previous versions of GiA) to get_iplayer 3.01 used in current (1.9bx) GiA versions.

The GiP coder, in his infinite wisdom, thought it better to change the tagging format and instead of using episode thumbnails to embed into M4A/MP4 files, he opted to using brand/series thumbnails; this was commit https://github.com/get-iplayer/get_iplayer/commit/ee97ef4e22620c490a1e9712a649fab92aeab1d7 from more than a year ago (it was present in GiP 2.95 release...). I, and some other GiP users, happen to disagree with that decision - some posted in the support forums and asked that the previous behaviour be restored, but the coder was adamant...

What I did locally was to locate inside get_iplayer 3.01 the piece of code that implements the change and comment it out (#); a similar route (i.e. reverting referenced commit) should be followed by current maintainer of GiA, that is if he is of the same opinion as I ...

skovatch commented 7 years ago

I am inclined to agree with the crowd here. The episode thumbnail is more interesting to me than the series thumbnail; I want to see what's on the web page for the show, not the series.

I hope this won't be a pain to keep track of, as I don't want to spend a lot of time merging perl code.

Vangelis66 commented 7 years ago

Yet another GiP (ex GiA) user reporting the same thing as the OP here: https://squarepenguin.co.uk/forums/thread-1415-post-6297.html ... to be greeted by the same negative response by the GiP coder!

@skovatch, maybe you could indeed create a PR in https://github.com/get-iplayer/get_iplayer/pulls If accepted and merged (doubtful, still) in a future GiP release, then "it wouldn't be a pain to keep track of" (assuming the future GiP release is consolidated into GiA).

skovatch commented 7 years ago

I actually did add a command-line and UI option that would let you switch between the two behaviors, since the logic for the old behavior is still there; he just overwrites the episode thumbnail URL, and fetches the series. I'm still testing it. Since it's turned on by a flag he might okay with it.

skovatch commented 7 years ago

This is now available in 1.9b7.

Vangelis66 commented 7 years ago

This is now available in 1.9b7.

Yes, I see it was commit https://github.com/Ascoware/get-iplayer-automator/commit/2db9877afaf61653b8089cea604b94b1a1e74f51 that actually implemented it :+1: . Huge thanks!

Since it's turned on by a flag he might okay with it.

Your perl code is very transparent, so I can patch my local copy of GiP 3.01 for use in my Windows laptop, but have you made your mind up yet about submitting a PR in https://github.com/get-iplayer/get_iplayer/pulls? The maintainer there is quite idiosyncratic, but, as you say, he might merge it in a future release, for the benefit of both GiP+GiA users...

Let me, once again, add my own voice to the praises already addressed to you for taking over this project...

Best regards

skovatch commented 7 years ago

I haven't gotten around to submitting a pull request over there, but it's clean enough that I would think he would take it. I'm more motivated to fix the updater, honestly., but I'll try to do it soon.

Vangelis66 commented 7 years ago

In some very rare occasions, and all three I've come across so far involve BBC video clips, GiP 3.01 with your patch throws up a (episode) Thumbnail download failed error and the resulting tagged MP4 file lacks the embedded thumbnail... Example clip is http://www.bbc.co.uk/programmes/p055hc6j, ergo --pid=p055hc6j This is a UK-only video-clip.

I did my own troubleshooting, it appears this is down to a BBC's fault:
pid=p055hc6j => JSON playlist =>http://www.bbc.co.uk/programmes/p055hc6j/playlist.json => holdingImageURL": "//ichef.bbci.co.uk/images/ic/$recipe/p055hdcw.jpg"

So, for a 640x360 thumbnail that would yield: http://ichef.bbci.co.uk/images/ic/640x360/p055hdcw.jpg => [HTTP/1.1 404 Not Found]

However, a thumbnail does exist, but with a different file extension, ".png": http://ichef.bbci.co.uk/images/ic/640x360/p055hdcw.png => [HTTP/1.1 304 Not Modified]

The thumbnail downloading perl code inside GiP 3.01 always assumes a ".jpg" file extension, hence the error generated and lack of thumbnail in end MP4 file (FWIW, AtomicParsley does support .png files for embedding).

The above was reported as more of an oddity rather than something that should be addressed by patching existing code...