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

Improve handling of incorrect paths in validator #119

Open
notartom opened this issue Apr 10, 2021 · 4 comments
Open

Improve handling of incorrect paths in validator #119

notartom opened this issue Apr 10, 2021 · 4 comments

Comments

@notartom
Copy link
Member

If a path is incorrect in the validator, this is not handled gracefully, and instead we get a whole bunch of errors because the following code in application/libraries/Librivox_mp3gain.php that parses the mp3gain output does not have a second line to work with:

$this->flag     = ' -o  ';
$command = $this->mp3gain .  ' ' . $this->flag . $dir. $file_name;
exec($command, $output);
$parts = explode("\t", $output[1]);

Here's a screenshot of an incorrect validator path (notice the period at the end of the first file):

Screenshot from 2021-04-10 14-56-14

And here are the PHP errors we get:

A PHP Error was encountered
Severity: Notice
Message: Undefined offset: 1
Filename: libraries/Librivox_mp3gain.php
Line Number: 73
A PHP Error was encountered
Severity: Notice
Message: Undefined index: playtime_string
Filename: libraries/Librivox_id3tag.php
Line Number: 171
A PHP Error was encountered
Severity: Notice
Message: Undefined offset: 2
Filename: libraries/Librivox_id3tag.php
Line Number: 172
A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 173
A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 177
A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 179
@twinkietoes-on
Copy link
Collaborator

This is happening frequently; a timely fix would be very helpful!

I also have this issue, the first two files are the guilty ones. I added them separately, the original two were renamed with the date, but now I can't edit the ID tags for the new ones (or for all in general) because of this error: All files must be linked to sections before you can edit the tags
https://librivox.org/validator/16345
Also, when I try to delete any file, all buttons freeze and I can't click anything, only after I refresh.

Capture2

A PHP Error was encountered
Severity: Notice
Message: Undefined offset: 1
Filename: libraries/Librivox_mp3gain.php
Line Number: 71

A PHP Error was encountered
Severity: Notice
Message: Undefined index: playtime_string
Filename: libraries/Librivox_id3tag.php
Line Number: 171

A PHP Error was encountered
Severity: Notice
Message: Undefined offset: 2
Filename: libraries/Librivox_id3tag.php
Line Number: 172

A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 173

A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 177

A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 179

A PHP Error was encountered
Severity: Notice
Message: Undefined offset: 1
Filename: libraries/Librivox_mp3gain.php
Line Number: 71

A PHP Error was encountered
Severity: Notice
Message: Undefined index: playtime_string
Filename: libraries/Librivox_id3tag.php
Line Number: 171

A PHP Error was encountered
Severity: Notice
Message: Undefined offset: 2
Filename: libraries/Librivox_id3tag.php
Line Number: 172

A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 173

A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 177

A PHP Error was encountered
Severity: Notice
Message: Undefined index: audio
Filename: libraries/Librivox_id3tag.php
Line Number: 179

notartom added a commit to notartom/librivox-catalog that referenced this issue Apr 16, 2021
In the validator, sometimes audio files either have a typo in their
path, or aren't actually audio files (when this patch was being
tested, the offending files were HTML files from a 301 redirect
response). In those cases, mp3gain does not return any useful output.
Specifically, instead of printing the header line plus the actually
useful line, it just prints the header line, with an error message
going to stderr (which we ignore because we only look at stdout).

This patch just makes the validator skip any such files.

Resolves LibriVox#119

Change-Id: Id08899ef6f415aeece6912e9676a7bcab05caf81
@notartom
Copy link
Member Author

notartom commented Apr 16, 2021

The change from the PR is on the server. @twinkietoes-on does it make sense? I'm basically just skipping any file that mp3gain cannot process for whatever reason.

notartom added a commit that referenced this issue Aug 28, 2021
In order to help with #119, we try to be smarter about the files we
import from the section compiler, and run an id3tag-based sanity check
on them. If they fail the check, we delete them from the validator,
and return an error message.
@notartom
Copy link
Member Author

Alright, I think the commit above (7f63d1c) should prevent broken files form reaching the validator. Is that the only problem though? What about the filenames with the period at the end? How do they end up happening?

@redrun45
Copy link
Collaborator

I was able to reproduce this one last night. A bug in a feature we don't seem to use anymore. This is much less urgent now that the warning messages don't interfere with AJAX requests, but worth fixing when the dust settles on other changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants