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

Tolerance margin for the sum of MO durations? #2093

Closed
rdeltour opened this issue Mar 18, 2022 · 7 comments · Fixed by #2106
Closed

Tolerance margin for the sum of MO durations? #2093

rdeltour opened this issue Mar 18, 2022 · 7 comments · Fixed by #2106
Labels
EPUB33 Issues addressed in the EPUB 3.3 revision Spec-EPUB3 The issue affects the core EPUB 3.3 Recommendation Topic-MediaOverlays The issue affects media overlays

Comments

@rdeltour
Copy link
Member

Since EPUB 3.3 the spec says:

The sum of the durations for each Media Overlay Document SHOULD equal the total duration.

EPUBCheck checks this, but I was wondering about using a tolerance margin. Should we define any in the spec? Or expect strict equality to the millisecond?

@rdeltour
Copy link
Member Author

@danielweck
Copy link
Member

Hello, in the various EPUB3 Media Overlays and DAISY SMIL parsers I implemented over time, the sum of individual durations from the reading order / publication spine (not accounting for the potentially-excluded time from skippable fragments) is compared against the declared metadata total duration. I also like to check the durations computed from the actual SMIL audio clip-begin/end pairs. Often, I observe discrepancies. If I remember correctly, I used a tolerance margin to mitigate rounding errors that can occur at authoring time, as well as in the reading system / validation tool's own code. I think I even went as far as allowing a 1-second margin of error. However note that this is really just developer-oriented information displayed as a console warning. In other words, discovered inconsistencies are not surfaced to the end-user, such as Thorium's "about publication" GUI panel for example.

I think it would be useful if EPUBCheck reported warnings for time differences greater than one second, which are unlikely to be the result of compound rounding errors, but rather of authoring / production mistakes. Would it be possible to check metadata durations (i.e. sum of declared individual spine item durations == declared total duration), as well as actual clip-begin/end SMIL pairs? (i.e. sum of audio durations in single SMIL file == declared metadata for individual spine item duration)

@rdeltour
Copy link
Member Author

I think it would be useful if EPUBCheck reported warnings for time differences greater than one second, which are unlikely to be the result of compound rounding errors, but rather of authoring / production mistakes.

Yeah, that's the question. Theoretically EPUBCheck tries to stick to the spec as close as possible, so I was wondering if this tolerance could be explicitly allowed in the spec.

Would it be possible to check metadata durations (i.e. sum of declared individual spine item durations == declared total duration), as well as actual clip-begin/end SMIL pairs? (i.e. sum of audio durations in single SMIL file == declared metadata for individual spine item duration)

Currently (latest preview) EPUBCheck checks metadata durations. Looking into the MO document to check that the declared duration matches the clips duration would be doable, although not a priority 😉 Feel free to file it to the EPUBCheck tracker!

@iherman
Copy link
Member

iherman commented Mar 18, 2022

Yeah, that's the question. Theoretically EPUBCheck tries to stick to the spec as close as possible, so I was wondering if this tolerance could be explicitly allowed in the spec.

+1 to that.

@mattgarrish mattgarrish added the Topic-MediaOverlays The issue affects media overlays label Mar 18, 2022
@OriIdan
Copy link

OriIdan commented Mar 19, 2022 via email

@mattgarrish
Copy link
Member

If we specify a tolerance, this means the sum MUST be within the tolerance.

Why would we need to change to a requirement?

If the recommendation becomes, for example:

The sum of the durations for each Media Overlay Document SHOULD equal the total duration plus or minus one second to account for rounding.

Then there is no warning whether you exactly match or are within the tolerance. Otherwise, you get a warning that the sum isn't within the expected tolerance, same as before.

@dauwhe dauwhe added the Agenda+ Issues that should be discussed during the next working group call. label Mar 23, 2022
@iherman
Copy link
Member

iherman commented Mar 25, 2022

The issue was discussed in a meeting on 2022-03-25

List of resolutions:

View the transcript

1. Tolerance margin for the sum of MO durations? (issue epub-specs#2093)

See github issue epub-specs#2093.

See github pull request epub-specs#2106.

Dave Cramer: we say the sum of the duration for each MO document should equal the total duration, but we've never specified a margin or tolerance for this.
… suggestion was that perhaps we should do so?.
… the suggestion was to allow 1 sec of tolerance before epubcheck would complain, to account for rounding errors.

Avneesh Singh: I think this is not controversial.

Ivan Herman: what I learned when I made tests, is that this value which is in the manifest is there to check the data.
… marissa has even suggested that this is unnecessary.

Ivan Herman: I can't say whether or not that is so, but if it is just for checking purposes, then allowing some rounding error is fine.
… not sure what the actual threshold should be, so for now I think 1 second is fine.

Zheng Xu (徐征): from what i've implemented, the duration is variable for different players on different platforms.
… so i wonder if this is even useful.

Avneesh Singh: regarding 1 second, i feel this is more than enough. Maximum 50 ms is what i've used in the past.
… for large audios (120 hours), a tolerance of 150 ms was enough.
… per my understanding, the purpose of stating this metadata is to allow RS to pull a "total duration" value into features like "you are 5 minutes into 12 total hours".

Brady Duga: can epubcheck just tell you the total duration value that should be in the manifest?.
… epubcheck can give you the right value, right?.

Matt Garrish: epubcheck is checking the total time, yes. I guess it could output that value..

GeorgeK: my recollection is that its difficult to calculate some of these things precisely, depending upon the compression that is being used, resulting in it being off by a smidgen in each file.
… so this type of tolerance seems absolutely fine, 1 second seems fine.

Ivan Herman: +1 to George.

Zheng Xu (徐征): as a RS I appreciate having the duration values for the whole book, and for each file.
… we use it as an indication of approximately where the user is, but it doesn't have to be that accurate.

Dave Cramer: i'm not hearing an argument against the 1 sec tolerance.
… it's obviously a useful piece of metadata for people reading the book.
… and having epubcheck able to tell if the sum = total is useful authoring tool, with 1 second tolerance.

Zheng Xu (徐征): +1.

Ivan Herman: +1.

Proposed resolution: Merge PR 2106, close issue 2093. (Wendy Reid)

Dave Cramer: +1.

Matt Garrish: +1.

Matthew Chan: +1.

Brady Duga: +1.

Masakazu Kitahara: +1.

Wendy Reid: +1.

Laura Brady: +1.

Avneesh Singh: +1.

Bill Kasdorf: +1.

Ivan Herman: +1.

GeorgeK: +1.

Zheng Xu (徐征): +1.

Ben Schroeter: +1.

Resolution #1: Merge PR 2106, close issue 2093.

Rick Johnson: +1.

@mattgarrish mattgarrish added the EPUB33 Issues addressed in the EPUB 3.3 revision label Mar 26, 2022
@mattgarrish mattgarrish removed the Agenda+ Issues that should be discussed during the next working group call. label Jul 2, 2022
@mattgarrish mattgarrish added the Spec-EPUB3 The issue affects the core EPUB 3.3 Recommendation label Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EPUB33 Issues addressed in the EPUB 3.3 revision Spec-EPUB3 The issue affects the core EPUB 3.3 Recommendation Topic-MediaOverlays The issue affects media overlays
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants