sandreas / tone

tone is a cross platform audio tagger and metadata editor to dump and modify metadata for a wide variety of formats, including mp3, m4b, flac and more. It has no dependencies and can be downloaded as single binary for Windows, macOS, Linux and other common platforms.
https://pilabor.com
Apache License 2.0
410 stars 17 forks source link

Tagging m4b is changing the timebase #32

Closed advplyr closed 1 year ago

advplyr commented 1 year ago

The milliseconds of chapter times are changing when tagging m4b files. I'm not sure if it is isolated to m4b because I haven't tested mp3 yet. This was reported here https://github.com/advplyr/audiobookshelf/issues/1060

When trying to track down why this is happening I noticed that m4b audiobooks I have that are using a 1/1000 timebase are being converted to 1/44100

I added a sample file named TimebaseSample.m4b here: https://github.com/advplyr/node-tone/tree/master/samples

Before tagging:

{
    "id": 1,
    "time_base": "1/1000",
    "start": 14043,
    "start_time": "14.043000",
    "end": 393113,
    "end_time": "393.113000",
    "tags": {
        "title": "Chapter 1"
    }
}

After tagging:

{
    "id": 1,
    "time_base": "1/44100",
    "start": 619296,
    "start_time": "14.042993",
    "end": 17336283,
    "end_time": "393.112993",
    "tags": {
        "title": "Chapter 1"
    }
}

The time base doesn't really matter in the end but I think it may be related to the milliseconds getting changed.

sandreas commented 1 year ago

The milliseconds of chapter times are changing when tagging m4b files.

Oh, this is bad... sorry. In my tests I used 1/44100 to ensure, that everything different from 1/1000 is parsed correctly. Maybe I missed something here - although I did not find any hard coded 44100 in the code that could be the reason for this behaviour. Maybe it's something different, either in ffmpeg or atldotnet...

This should be investigated and in my opinion there is no good reason to use something different from 1/1000 because this would be irritating and more difficult to map, although it should be ensured that there is no shift caused by rounding errors in audio when re-tagging. Maybe one could argue that high precision timers could need e.g. 1/1000000, but I think by default everything should be normalized to 1/1000 and only if let's say --meta-timebase is given, it should:

What do you think? And could you please report the exact command used to reproduce the issue?

advplyr commented 1 year ago

I don't think it has to do with ffmpeg since I'm only using ffprobe to read the data. Here is how to reproduce with the sample file

  1. ffprobe -i TimebaseSample.m4b -show_chapters -print_format json (note chapters with 1/1000 timebase)
  2. tone tag TimebaseSample.m4b --meta-title "Hello World"
  3. ffprobe -i TimebaseSample.m4b -show_chapters -print_format json (chapters now with 1/44100 timebase)

It may be altdotnet. I don't know enough about audio encoding to say if it matters to use 1/1000. The timebase is usually based on the sample rate so this may allow for more accuracy?

One issue with changing the timebase seems to be it is changing the chapter start/end times.

sandreas commented 1 year ago

I don't think it has to do with ffmpeg since I'm only using ffprobe to read the data.

No this is definitely a tone issue. Thank you for your patience.

Here is how to reproduce with the sample file

Great thanks, I'll have a look asap.

I don't know enough about audio encoding to say if it matters to use 1/1000. The timebase is usually based on the sample rate so this may allow for more accuracy?

Neither do I. I have to do some research, what the problem is here but this is indeed an interesting problem, since I found NO hint that tone does something to change the timebase of the chapters. The 44100 just came to my mind because I also got a sample file for my tests to ensure, that values different from 1/1000 also do work as expected.

Now that I can reproduce the issue (hopefully), I'm sure I find out pretty quick, what the problem is.

One issue with changing the timebase seems to be it is changing the chapter start/end times.

That should never happen... it is definitely a problem, even if it is only a rounding issue.

sandreas commented 1 year ago

I reproduced the problem and I verified it is probably atldotnet - the reference issue is #166, let's see what happens next :-)

sandreas commented 1 year ago

Ok, atldotnet released a fix for this today. I try to integrate this in the next days...