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

renaming chapters doesn't rename all chapters #24

Closed
MrBrN197 opened this issue Sep 12, 2022 · 9 comments
Closed

renaming chapters doesn't rename all chapters #24

MrBrN197 opened this issue Sep 12, 2022 · 9 comments

Comments

@MrBrN197
Copy link

I have a mb4 file that has chapters title 1-67. I created a chapters.txt with the correct names and used the same times from tone dump. however the last for chapters are not renamed. here is --dry-run preview.

using this command tone tag file.m4b --auto-import=chapters --dry-run

image

this is the relevant section in the chapters.txt file

image

is this a bug or am I missing something?

@sandreas
Copy link
Owner

sandreas commented Sep 12, 2022

is this a bug or am I missing something?

It might be :-) Would you mind providing the full chapters.txt file? I have a test m4b file with 27hours, so I could do an import of your chapters file to reproduce the problem.

@MrBrN197
Copy link
Author

chapters.txt

@sandreas
Copy link
Owner

Thank you. I found the problem:

C# has a TimeSpan class that has a method TryParse to parse time strings. This class, as the name states, is used to represent a timespan and also can also be used to represent timespans greater than 24 hours (why not?). However, unfortunately the TryParse method seems to be limited and not to support timespans > 24h (WTF?).

This (currently) results in any chapter after 24h to be skipped, when using the chapters.txt format.

Next steps:

As a workaround you could maybe try the tone.json format to rework your chapters, but I have to check, if this format suffers the same issue.

tone dump --format=json "my-file.m4b" > tone.json
# do modifications
tone tag my-file.m4b --meta-tone-json-file="tone.json" --auto-import="tonejson"

@sandreas
Copy link
Owner

Should be fixed. Targeted for release v0.1.0.
The check for other formats having this issue is not fully done yet, but my first impression is, that other formats should work. Please try the tone.json workaround, if you find the time.

@MrBrN197
Copy link
Author

I see thanks for the quick fix. I see you do the hours separately

@sandreas
Copy link
Owner

I see thanks for the quick fix. I see you do the hours separately

Yes, it seems that TimeSpan.TryParse does support >24h strings but not in the format 27:34:59.000. It has to be 1.03:34:59.000 for 1 day, 3 hours, 34 minutes, 59 seconds and 0 milliseconds. However, this is not the format given in the chapters.txt file, so it has to be handled in a different way.

@MrBrN197
Copy link
Author

I don't know C# but you could also add the days part into any entries that don't have days Before you get to the TryParse. that could eliminate the need for the edge case you created.

Thanks for creating this tool

@sandreas
Copy link
Owner

I don't know C# but you could also add the days part into any entries that don't have days Before you get to the TryParse. that could eliminate the need for the edge case you created.

Thank you. Nice idea, but unfortunately, this won't work. The problem is, that TryParse does not support values for the hour part, that are greater 24. So if I add the days part with e.g. this value: 0.27:00:00.000 or 1.27:00:00.000, it still could not be parsed. Values > 24 are just invalid and so you have to parse the string manually anyways. In the case of chapters.txt the format is fixed, so I don't need a more generic approach and I always try to keep things simple. If I would need it anywhere else, I think I would write my own parse and toString logic with a unit test.

Thanks for creating this tool

You're welcome... spread the word ;)

@MrBrN197
Copy link
Author

yeah, not sure what I was thinking .. that's quite a headache. cheers 👍

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

No branches or pull requests

2 participants