Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Call to a member function comment() on null #18

Closed
cospin opened this issue Oct 30, 2023 · 10 comments
Closed

[Bug]: Call to a member function comment() on null #18

cospin opened this issue Oct 30, 2023 · 10 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@cospin
Copy link

cospin commented Oct 30, 2023

What happened?

It seems that when an .mp3 file only has ID3v2 information and no v1, the following error is generated:

[30-Oct-2023 21:08:56 UTC] PHP Fatal error:  Uncaught Error: Call to a member function comment() on null in site\vendor\kiwilan\php-audio\src\Models\AudioCore.php:430
Stack trace:
#0 sitet\vendor\kiwilan\php-audio\src\Audio.php(395): Kiwilan\Audio\Models\AudioCore::fromId3(NULL, Object(Kiwilan\Audio\Models\Id3AudioTagV2))
#1 site\vendor\kiwilan\php-audio\src\Audio.php(98): Kiwilan\Audio\Audio->parse()
#2 site\upload.php(112): Kiwilan\Audio\Audio::get('site/...')
#3 site\upload.php(407): newMetadata('site/...', 'Miss K8 - Infin...')
#4 {main}
  thrown in site\vendor\kiwilan\php-audio\src\Models\AudioCore.php on line 430

The relevant line is:

comment: $v2->comment() ?? $v1->comment(),

In this case, the function is being called as: fromId3(NULL, Object). Replacing the line with: comment: $v2 ? $v2->comment() : $v1->comment(), works.

How to reproduce the bug

For now I think it happens when a .mp3 file only has only ID3v2 information and no v1.

Package Version

3.0.02

PHP Version

8.2.11

Which operating systems does with happen with?

Windows

Notes

Really nice work, thank you!

@cospin cospin added the bug Something isn't working label Oct 30, 2023
@ewilan-riviere
Copy link
Contributor

Thanks, I will fix this today 👍

@ewilan-riviere ewilan-riviere added the good first issue Good for newcomers label Oct 31, 2023
ewilan-riviere added a commit that referenced this issue Oct 31, 2023
ewilan-riviere added a commit that referenced this issue Oct 31, 2023
- `AudioCore`, `fromId3()` method comment bug, issue #18
@ewilan-riviere ewilan-riviere mentioned this issue Oct 31, 2023
@ewilan-riviere
Copy link
Contributor

Can you check latest version?

@cospin
Copy link
Author

cospin commented Nov 1, 2023

@ewilan-riviere unfortunately, I found another file that gives a similar error, but this time it's Call to a member function genre() on null. I suppose we need to make sure that $v2 exists in every field, not just in the comment field.

To be honest, it's somewhat unusual, and without looking at the code a bit, I wouldn't know why it's happening. It's supposed to be the case that $v2 should never be null in fromId3(?Id3AudioTagV1 $v1, Id3AudioTagV2 $v2), and the trace shows that it's called with $v2: fromId3(NULL, Object(Kiwilan\Audio\Models\Id3AudioTagV2))

This time, it happened with a Tag ID3v2.4 (ID3v2.4), which has several empty fields, including the genre.

Checking for $v2 as we do with comment works, but seems weird to have to check $v2 on every field.

@ewilan-riviere
Copy link
Contributor

I will fix this, if you have a sample to provide for tests (if you can share it), it would be great!

@ewilan-riviere
Copy link
Contributor

Thanks! Can I let these files into tests files?

@cospin
Copy link
Author

cospin commented Nov 1, 2023

No, they are not mine and have copyright 🙃
Let me know if you have downloaded them so I can remove the link.

@ewilan-riviere
Copy link
Contributor

ewilan-riviere commented Nov 1, 2023

No problem, I will create sample from their metadata.
Yes, you can remove it

ewilan-riviere added a commit that referenced this issue Nov 1, 2023
- `AudioCore`, fix `fromId3()` with `null` check `Id3AudioTagV1` and `Id3AudioTagV2`, issue #18 thanks to @cospin
@ewilan-riviere ewilan-riviere mentioned this issue Nov 1, 2023
@ewilan-riviere
Copy link
Contributor

Can you test with latest version?

@cospin
Copy link
Author

cospin commented Nov 1, 2023

Yep, seems to work fine. Thank you! 🙏

@ewilan-riviere
Copy link
Contributor

No problem, tell me if you find another bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants