Thank you for your donation!


Cloudsmith graciously provides open-source package management and distribution for our project.


Solved: Cover images not loaded for all albums
#21
(05-11-2023, 12:06 PM)TheOldPresbyope Wrote: @haeckse
@Nutul 
@Tim Curtis 

What Tim said.

Sorry, I just noticed one last issue with the Show Me Something Good track you shared. Even after I converted to ID3 V2.3, removed the comments, and retyped the APIC frame as type 03 Cover (front) I was getting a thumbnail but not a full cover displayed during playback.

The track metadata is declaring the APIC format to be MIME type "image/PNG". I haven't had my coffee this morning so haven't browsed the code, but might we not be flattening upper- to lowercase when processing embedded images? Anyway, as a quick-n-dirty fix I edited the MIME type in the metadata to "image/png", and the cover now displays.

Regards,
Kent

Hi Kent,

had another look at the code, and:

- the Zend library looks at the APIC frame, doesn't seem to check for specific ImageType (although the image type IS read and stored in the APIC frame), so probably the first found image is used.

- the MIME type is returned as-is

- the thumbnail generator, to output the image, expects the MIME type to be all lowercase; so if the MIME is indeed "image/PNG", then it won't be matched, and no image is returned (even if it is there...), as we hit the default switch case, which just breaks' out.

Changing switch($mime) to switch(strtolower($mime)) in the code could help...
Reply
#22
(05-11-2023, 12:06 PM)TheOldPresbyope Wrote: @haeckse
@Nutul 
@Tim Curtis 

What Tim said.

Sorry, I just noticed one last issue with the Show Me Something Good track you shared. Even after I converted to ID3 V2.3, removed the comments, and retyped the APIC frame as type 03 Cover (front) I was getting a thumbnail but not a full cover displayed during playback.

The track metadata is declaring the APIC format to be MIME type "image/PNG". I haven't had my coffee this morning so haven't browsed the code, but might we not be flattening upper- to lowercase when processing embedded images? Anyway, as a quick-n-dirty fix I edited the MIME type in the metadata to "image/png", and the cover now displays.

Regards,
Kent

Kent,

I can confirm your findings.

Those APIC type/MIME type settings displayed thumbnails as well as the full scale cover at playback:

FRONT_COVER Image: [Size: 58684 bytes] [Type: image/png]
FRONT_COVER Image: [Size: 260249 bytes] [Type: image/jpeg]

while those didn't:

OTHER Image: [Size: 50435 bytes] [Type: image/jpeg]
FRONT_COVER Image: [Size: 260249 bytes] [Type: image/JPEG]
FRONT_COVER Image: [Size: 58684 bytes] [Type: image/PNG]

So setting the image as type "front cover" plus a lowercase MIME type does the trick. Though it is a mystery to me why when reading my entire song library, the covers of a large number of files are displayed even though the APIC type is set to "other".

I can't think of an easy method to fix my entire library in batch mode yet. A quick look into the eyed3 params didn't look promising. But there is pretty verbose documentation for the tool that I will look into.

I have no idea about the complexity yet, but maybe it's worth investing some time diving into the Moode code to understand how and where the ID3 data is processed. Maybe I can manage to make the parser more fault-tolerant in dealing with messy metadata, which will probably be more fun than plodding through fixing the bad tags : )

Regards,
Andrea
Reply
#23
@haeckse 

Thanks for that, I was beginning to think I was hallucinating.

So far I've been hacking my test cases with a combination of eyeD3 from the command line and kid3 via its GUI just because they were on my mind.

That's ok for a couple of tracks but the process needs to be scripted. Thankfully there's the companion kid3-cli. Probably still need eyeD3 because kid3 doesn't handle ID3v2.2 -> ID3v2.3 AFAIK. There's other tools available but let's not make this more complicated unless we have too. 

How many tracks do you think will need correction?

Regards,
Kent
Reply
#24
(05-11-2023, 07:18 PM)haeckse Wrote: OTHER Image: [Size: 50435 bytes] [Type: image/jpeg]
FRONT_COVER Image: [Size: 260249 bytes] [Type: image/JPEG]
FRONT_COVER Image: [Size: 58684 bytes] [Type: image/PNG]
(...)
I have no idea about the complexity yet, but maybe it's worth investing some time diving into the Moode code to understand how and where the ID3 data is processed. Maybe I can manage to make the parser more fault-tolerant in dealing with messy metadata, which will probably be more fun than plodding through fixing the bad tags : )

See my previous post.

OTHER Image should work because the APIC frame parser is not so strict to look for specific pictures (I am not sure, but in case many APIC frames are present, the first one will be returned... whatever its type is). I am a bit rusty on PHP, but I understand the following code adds an entry to an array:

$this->_frames[$frame->getIdentifier()][] = $frame
;

so that $this->_frames['APIC'] is an array of all the 'APIC' frames found in the id3v2 tag.

id3v2->apic (which is what the moOde's image handler looks for) is a proxied method retriever that looks into the frame IDs, and it returns the FIRST frame with such ID ('apic') converted to all capitals:

Code:
public function __get($name)
{
  if (!empty($this->_frames[strtoupper($name)])) {
          return $this->_frames[strtoupper($name)][0];
      }

the mixed case mime types image/PNG, or image/JPEG, won't work because moOde's thumbnail generator / image extractor checks the MIME type against lowercase strings only, e.g. 'image/png', 'image/jpeg', etc.
Reply
#25
Looks like that "lowercase" mime type issue needs to be fixed.

I'll look into it for upcoming 8.3.3 release.
Enjoy the Music!
moodeaudio.org | Mastodon Feed | GitHub
Reply
#26
(05-11-2023, 08:12 PM)TheOldPresbyope Wrote: @haeckse 

Thanks for that, I was beginning to think I was hallucinating.

So far I've been hacking my test cases with a combination of eyeD3 from the command line and kid3 via its GUI just because they were on my mind.

That's ok for a couple of tracks but the process needs to be scripted. Thankfully there's the companion kid3-cli. Probably still need eyeD3 because kid3 doesn't handle ID3v2.2 -> ID3v2.3 AFAIK. There's other tools available but let's not make this more complicated unless we have too. 

How many tracks do you think will need correction?

Regards,
Kent

I suppose there are roughly 4000 files that need to be corrected.

As for the "other" or "front cover" problem,  I'm not quite sure anymore whether everything went right in my tests in view of Al's code research. Especially because the cover appears with many files with "other", Al's findings are plausible. I will test this again tomorrow with a handful of files.

Regards,
Andrea
Reply
#27
(05-11-2023, 08:12 PM)Nutul Wrote:
(05-11-2023, 07:18 PM)haeckse Wrote: OTHER Image: [Size: 50435 bytes] [Type: image/jpeg]
FRONT_COVER Image: [Size: 260249 bytes] [Type: image/JPEG]
FRONT_COVER Image: [Size: 58684 bytes] [Type: image/PNG]
(...)
I have no idea about the complexity yet, but maybe it's worth investing some time diving into the Moode code to understand how and where the ID3 data is processed. Maybe I can manage to make the parser more fault-tolerant in dealing with messy metadata, which will probably be more fun than plodding through fixing the bad tags : )

See my previous post.

OTHER Image should work because the APIC frame parser is not so strict to look for specific pictures (I am not sure, but in case many APIC frames are present, the first one will be returned... whatever its type is). I am a bit rusty on PHP, but I understand the following code adds an entry to an array:

$this->_frames[$frame->getIdentifier()][] = $frame
;

so that $this->_frames['APIC'] is an array of all the 'APIC' frames found in the id3v2 tag.

id3v2->apic (which is what the moOde's image handler looks for) is a proxied method retriever that looks into the frame IDs, and it returns the FIRST frame with such ID ('apic') converted to all capitals:

Code:
public function __get($name)
{
  if (!empty($this->_frames[strtoupper($name)])) {
          return $this->_frames[strtoupper($name)][0];
      }

the mixed case mime types image/PNG, or image/JPEG, won't work because moOde's thumbnail generator / image extractor checks the MIME type against lowercase strings only, e.g. 'image/png', 'image/jpeg', etc.


Maybe I confused myself somehow in my tests regarding the "other image" issue. I will rerun my tests tomorrow.

As for the MIME-type problem, it's nice that you found the root cause, which seemingly can be solved relatively easily. Thanks for looking into the code and the explanation! 

Regards,
Andrea
Reply
#28
@haeckse 

I'm getting some confusing results concerning thumbnails (but not covers) here too but partly that's cuz I'm trying to work too many projects at once.

Applying the principle of least work (that's a thing, actually, for us physicists), I'd say stick at the moment with converting the ID3 tag version and removing the comments. Wait for the code jockeys to agree a fix on the PHP code relating to coverart and thumbnail generation before seeing if you need to change anything else.

And enjoy the music!

Regards,
Kent
Reply
#29
Ah, got it. After separating my multiple test files, it’s clear that fixing the image/PNG issue is sufficient. As Al pointed out, the library routines grab the embedded image regardless of its type. That was masked by my mixing of files. Mea culpa.

Regards,
Kent
Reply
#30
(05-11-2023, 10:39 PM)TheOldPresbyope Wrote: @haeckse 

I'm getting some confusing results concerning thumbnails (but not covers) here too but partly that's cuz I'm trying to work too many projects at once.

Applying the principle of least work (that's a thing, actually, for us physicists), I'd say stick at the moment with converting the ID3 tag version and removing the comments. Wait for the code jockeys to agree a fix on the PHP code relating to coverart and thumbnail generation before seeing if you need to change anything else.

And enjoy the music!

Regards,
Kent


@TheOldPresbyope

I totally agree, it is best to wait for the fix and doing some other fun stuff instead. Like connecting a rotary encoder to the Pi's GPIO pins for volume control. Let's see how that goes Smile

Thank you for your dedication to tracking down this issue!

Regards,
Andrea
Reply


Forum Jump: