File loop problem

Kirby 3 Solved ✅
1

So I’m putting some finishing touches to a plugin ive working in and I’ve run into a weird issue when i use it in file loops. It claims the track index in the array does not exist, but if i hit the file directly, it works!!

If i do this, i can get the track information just fine:

<?= $page->file('01-signs-of-life.mp3')->id3('track')?>

If i do this, it tells me the track index does not exist. it DOES work if there is only 1 mp3 file in the content folder. if its looping over more then one, it fails :frowning:

<?php foreach($page->audio() as $mp3): ?>
      <div class="album">
      <img src="<?= $mp3->id3('cover')?>">
        <ul>
          <li><?= $mp3->id3('album')?></li>
          <li><?= $mp3->id3('artist')?></li>
          <li><?= $mp3->id3('title')?></li>
          <li><?= $mp3->id3('track')?></li>
          <li><?= $mp3->id3('trackno')?></li>
          <li><?= $mp3->id3('composer')?></li>
          <li><?= $mp3->id3('genre')?></li>
          <li><?= $mp3->id3('year')?></li>
          <li><?= $mp3->id3('duration')?></li>
        </ul>
      </div>
<?php endforeach ?>

Heres my whole plugin…

<?php
require('lib/getid3/getid3.php');

Kirby::plugin('hashandsalt/id3', [
	'fileMethods' => [
		'id3' => function ($mediainfo = 'title') {

			$getID3 = new getID3;
			$mediaid = $getID3->analyze($this->root());

			getid3_lib::CopyTagsToComments($mediaid);

			$title = $mediaid['comments_html']['title'];
			$artist = $mediaid['comments_html']['artist'];
			$album = $mediaid['comments_html']['album'];
			$genre = $mediaid['comments_html']['genre'];
			$yearraw = $mediaid['comments_html']['year'];
			$year = implode('', $yearraw);
			$composer = $mediaid['comments_html']['composer'];
			$duration = $mediaid['playtime_string'];
			$coverimg = $mediaid['comments']['picture'][0]['data'];
			$coverdata = base64_encode($coverimg);
			$cover = 'data:image/jpeg;base64,'.$coverdata;
			$tracknum = $mediaid['comments_html']['track'];
			$tracknumof = $mediaid['comments_html']['track_number'];


			$mediadata = array(
				'title' => implode('', $title),
				'artist' => implode('', $artist),
				'album' => implode('', $album),
				'genre' => implode('', $genre),
				'year' => substr($year, 0, 4),
				'composer' => implode('', $composer),
				'duration' => $duration,
				'track' => implode('', $tracknum),
				'trackno' => implode('', $tracknumof),
				'cover' => $cover,
			);

			return $mediadata[$mediainfo];

			// return $mediaid;

		}
	]
]);

Here is the array im pulling info from:

array(10) {
  ["title"]=>
  string(13) "Signs of Life"
  ["artist"]=>
  string(10) "Pink Floyd"
  ["album"]=>
  string(27) "A Momentary Lapse of Reason"
  ["genre"]=>
  string(4) "Rock"
  ["year"]=>
  string(4) "1987"
  ["composer"]=>
  string(23) "David Gilmour/Bob Ezrin"
  ["duration"]=>
  string(4) "4:24"
  ["track"]=>
  string(1) "1"
  ["trackno"]=>
  string(4) "1/10"
  ["cover"]=>
  string(20147) "data:image/jpeg;base64"
}
2

Are you sure all the files have all the information? Also, it seems a bit weird that your plugin always generates all the data just to return a singe value? There’s also a lot of duplicate code…

3

If i dont do that, I end up with nested arrays. I had to build it up and flatten it with implode. If theres a better way, Id love to understand it :slight_smile:

And yep… the info is there in the file, and it works in a loop with many files if i dont ask for the track number.

4

IMHO, your plugin should look something like this:

require('lib/getid3/getid3.php');

Kirby::plugin('hashandsalt/id3', [
	'fileMethods' => [
         'id3' => function ($mediainfo = 'title') {

         $getID3 = new getID3;
         $mediaid = $getID3->analyze($this->root());

         getid3_lib::CopyTagsToComments($mediaid);
         // let's make sure the index exists, otherwise return an empty array
         $comments = $mediaid['comments_html']?? [];
         // same here
         return implode('',$comments[$mediainfo]?? []);
		}
	]
]);

Since you only want that one key, don’t go through the whole thing…

You probably need an additional if statement for the coverstuff…

5

But not all of it is in the comments_html array. The playtime and the cover image are in other parts of the ID tag, but i guess i could append those.

6

Well, then check what the key is and get the stuff from that other array, but really, all that stuff is completely unnecessary. Or yes, append it, probably even better, but keep it short and limited to what you need.

But more importantly, make sure that you have a fallback if the index doesn’t exist. Otherwise you run into issues when looping through multiple files where you don’t know what they contain. It works with your single file, but not with the rest.

7

Well yes it probably is unnecessary but we learn by our stupidity :slight_smile:

The odd thing is that I don’t know why its failing, i’ve checked all the mp3’s i’ve been testing with and they have track data stored in them. It’s just nuts.

Im also mulling over wether this should be a file method at all and might be better as a file hook that stamps the meta data with this information instead, on upload & replace. It actually takes 2 or 3 seconds to fetch the entire ID tag into an array, so maybe the problem is speed.

8

That’s sounds like a better idea, actually. As long as it doesn’t slow the Panel down too much.

9

Ok i think this is much better and the track seems to work now too…

Kirby::plugin('hashandsalt/id3', [
	'fileMethods' => [
		'id3' => function ($mediainfo = 'track') {

			$getID3 = new getID3;
			$mediaid = $getID3->analyze($this->root());

			getid3_lib::CopyTagsToComments($mediaid);

			// Check existence...
			$comments = $mediaid['comments_html']?? [];

			// Check for Album Artwork
			if ($coverimg = $mediaid['comments']['picture'][0]['data']?? []) {
				$coverdata = base64_encode($coverimg);
				$coverbase = 'data:image/jpeg;base64,'.$coverdata;
			}	else {
  			$coverbase = '';
			}

			// Duration and Artwork
			$otherinfo = array(
				'cover'		=> array($coverbase),
				'duration' 	=> array($mediaid['playtime_string']),
			);

			// Merge all the info
			$audioinfo = array_merge($comments, $otherinfo);

			return implode('', $audioinfo[$mediainfo]?? []);
		}
	]
]);
10

I think it would be more performant to return an array or even better a collection with all the information your need from your file method. Then you have to call the method only once instead of for each little piece of information, store the results in a variable (($meta = $file->id3())and take it from there.

11

Im not sure I follow you. Im already only getting what i need and storing it in an array, and then working off it. I don’t get the difference.

12

Currently, you get the information by passing a parameter to your function, something like artist. What you get back from your method is the artist. Then next you want the track, that means another call to your method, another time analyzing the file and getting the track from the data. And then again for the next piece of information and so on. Instead, you could return the array (ideally as a collection), then in your template, loop through the collection and output each piece of information.

13

I see… i take it i can do a collection in a plugin? I was just reading the docs for plugins and collections and I cant see any mention of it.

Also, if i wanted to write this information into the meta file, how I would I do that? I know how to do the hook in the plugin, i just want to know how to write the information to the file.

14

Check out

$file->update()
1 Like
15

if you notice a drop in performance from the repeated calls to the mp3 metadata you could only call it once and cache the result.
just wrap everthing inside of your filemethod with my lapse plugin callback. use the md5($file->id().$file->modified()) as cache id and you are done.

16

Thanks @bnomei i but I dont really want to depend on another plugin from my plugin :slight_smile:

17

I’d still argue that if you need all the information from the array anyway that repeated calls to the method are unnecessary.

1 Like
18

then steal the cache implemntation. its really simple. :wink:

19

Not for mere mortals like me it isn’t… im a front end / designer guy… I think the collection idea might be the way to go.

20
1 Like