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

wrong duration-ETA for some voice-note audiofiles, until after playback #3116

Closed
1 task done
five-c-d opened this issue Feb 7, 2019 · 3 comments
Closed
1 task done

Comments

@five-c-d
Copy link

five-c-d commented Feb 7, 2019

  • I have searched open and closed issues for duplicates

Bug Description

Some audiofiles show the incorrect duration on signal4desktop, for example, it will claim the voice-note I just received is 1:00 long, but when I play it there are 1:22 seconds of actual audio-playback.

Steps to Reproduce

  1. install signal4desktop beta on linux
  2. receive a voice-note as an .AAC file-attachment from signal4android beta
  3. look at the estimated duration in the message-bubble, it says "1:00"

Actual Result:

  1. press play, and watch as the playback goes past 1:00, all the way to 1:22
  2. the duration is correct thereafter in the chat-bubble (until restart)
  3. hover on the chat-bubble, more info, still claims it is 1:00
  4. close signal4desktop, and re-open it, and 1:00 appears in the chat-bubble again

Expected Result:
message-bubble should correctly estimate 1:22 as the playback time, from the beginning

Screenshots

Platform Info

Signal Version: 1.18.0-beta.7

Operating System: Arch Linux (installed signal4desktop via AUR)

Linked Device Version: signal4android 4.32.8 running android 7.1.1 (device that send the voice-note-files was signal4android 4.33.x beta-channel running android 8.x however)

Link to Debug Log

https://debuglogs.org/3093348aa060b02eb7e15177b65ba345f85468b9fad0f9a19c330a28b9816aae

  • INFO 2019-02-06T21:01:20.208Z data message from +[REDACTED]185.1 1549486875648 (c8811ae7-0d6b-4ca1-abca-c6a5f4d64842) ... claims 1:30 , actually 1:30 == ok
  • INFO 2019-02-06T21:01:56.092Z data message from +[REDACTED]185.1 1549486911879 (435a10ac-4d76-4971-bd00-814826c235bd) ... claims 0:27 , actually 0:33 == wrong
  • INFO 2019-02-06T22:39:33.953Z data message from +[REDACTED]185.1 1549492769205 (38f078c1-4a63-43b6-b3c9-41606fcb3edb) ... claims 1:00 , actually 1:22 == wrong

The above are the 3 voice-note audiofiles in this debuglog.

  • when I downloaded Minor updates to curve255.js #1, file-properties claim 1:29 duration, MPEG-4 container, AAC audio-codec, 44khz stereo, 388KB. before playing, signalapp claimed 1:30 duration (right), after playing 1:30 duration, after restarting signal4desktop still says 1:30.
  • when I downloaded Fix the clickies #3, file-properties claim 1:21 duration, MPEG-4 container, AAC audio-codec, 44khz stereo, 354KB. before playing, signalapp claimed 1:00 duration (wrong), after playing 1:22 duration, after restarting signal4desktop says 1:00 again (back to wrong)

Background

According to this comment back in 2017 signal4android was sending mono AAC, rather than stereo AAC. Not sure that matters for duration-estimates

Potentially helpful background info: #2883 , which discusses the variety of audio-codecs that are being used for all the different signalapp flavours, when sending and receiving voice-notes.

Not the same issue as #1496 , which is discussing desired shrinkage of filesize of the voice-notes when downloaded... whereas this bug-report is about the GUI-label which estimates the playback-duration.

Upstream bugs similar to this one

discussion of chromium flaws, and possible workarounds

Seems very likely, since the duration-estimate is coming from an HTML5 <audio> tag used for rendering voice-note attachments/controls, that the root cause of the problem is a bug in chrome. I didn't find this specific problem mentioned (some AAC files having estimated playback-duration ~80% of actual playback-duration), but there are plenty of chromium bugs over the years with incorrect estimations of various sorts of audio-files.

Reading between the lines, from the way that the chromium devs respond in the bug-tracker, if it works in chrome on windows, they do not put a high priority on bugfixing this type of thing, since they expect "nobody" will run into it in their target audience.

For signalapp the problem is more severe, because one of the major "strategies" used to interact with signalapp is people that send each other async voice-notes. Not everybody does this, and not everybody who does, utilized signal4desktop, but it is more of a typical-use-case methinks (whereas running chrome on non-windows systems is a corner-case). Interesting, signal4android does not suffer from any incorrect-duration-estimations ... but that is because it does not display any estimates! Signal4desktop is more advanced in this specific feature, and does display duration-estimations, but because chromium sometimes gets them wrong, signal4desktop is also getting them wrong (unless I'm getting the root cause wrong).

Fixing the problem, assuming that fixes to the upstream chromium codebase will not be forthcoming any time soon.... Perhaps the <audio> tag can be messed with via the HTMLMediaElement parent and custom controls with a custom-calculated-duration-estimate finagled into place, as a label? The helpdocs seem to suggest that would not work though; they treat the duration as a file-coded-value.

So, I suspect that the 'fastest' way to help solve the immediate problem, would be to have signal4android annotate the .M4A datafile before sending it over, with some metadata tags which specify the duration (in a way that the chromium-version-within-electron picks up). On x86 linux systems this would usually be done with easytag or libmp4m2 or similar... not sure about what libraries would do that on an android system. Possibly changing the codecs various flavours of signalapp utilize for voice-notes would help, alternatively, since the chromium-duration-estimate bugs seem to be file-format-specific for the most part.

@scottnonnenberg-signal
Copy link
Contributor

Quite an in-depth investigation! Thank you! I do suspect that our best way around this is to include the duration alongside the audio file. But today we hand off the audio file to an <audio> tag entirely for rendering, so we'd also be signing up for custom rendering of audio controls.

Essentially: a lot of work for something that isn't a particularly severe effect and doesn't happen that often? We probably won't get to this any time soon. :0/

@five-c-d
Copy link
Author

five-c-d commented Feb 7, 2019

My suspicion, is that by changing to a different codec, or perhaps merely by changing the exact m4a metadata-tags that are annotating voice-notes in the current fileformat/codec, we would eventually hit upon a combo that WAS accepted (and properly rendered) by the 'captive' chromium which signal4desktop is able to use.

In other words, by fiddling around with different sending-fileformats and sending-tag-metadata, I expect you would eventually hit upon a combination that signal4android CAN properly generate/send, and also that the chromium bundled with signal4desktop CAN render with the default <audio> controls (and no special kludges). Because you control both ends of the conversation, in this case, something is likely to "just work". Can greyson send MP3, instead of AAC-wrapped-in-MP4, perhaps? Chromium seems to give correct duration-estimates for that fileformat.

This bug-report would still need to remain open... even if a codec-change helps voice-notes... because in addition to voice-notes, an enduser might send an arbitrary audio-file as an attachment. But I think the problems are separate: in the first case, when signal4android sends a voice-note to signal4desktop, we want the codec and tags selected by signal4android to work with signal4desktop. The second use-case is where the enduser is choosing to send AAC-in-M4A as an audio-file attachment ... and in that (rarer) situation, signal4desktop would display the wrong duration-estimate. But that's not as bad as displaying the wrong duration-estimate for voice-notes, because it would happen more rarely, and because endusers would not be as quick to "lay blame" on signalapp for failing to work right.

Even though it is chromium-in-electron not working right... endusers usually don't see the difference, though of course, there is a valid distinction, here on github :-)

Point being: if you end up messing around with codec-choices, as part of solving existing feature requests 2883 and 1496 mentioned above, please add solving this 3116 bug-report as a secondary goal of those codec-replacement efforts, iff possible.

p.s. Signal4desktop has trouble uploading some kinds of imagefiles that are rarely-seen fileformats... https://github.com/kripken/j2k.js/blob/master/relax.jp2 which is JPEG2000 for example, gives "Unable to load selected attachment" visually and /opt/Signal Beta/resources/app.asar/js/logging.js:103 ERROR 2019-02-07T01:45:54.333Z Error ensuring that image is properly sized: Event {isTrusted: true, type: "error", target: img, currentTarget: img, eventPhase: 2, …} in the debuglog.

The workaround is to put that into a zipfile, or just temporarily give it a fake-file-extension, at which point signal4desktop can attach it. And of course, almost nobody needs to mess with that fileformat :-) So I have not filed a github report. But plenty of people send voice-notes, and while being off by 20% of the duration is no big deal for a one-minute audio-clip, for longer ones it could be a problem someday. In the short run I think there is a 15-minute cap on voice-note durations, though, so it would be just an annoyance-bug rather than one that impacts usability.

Agree with the defer-until-later strategy, in other words :-) There are bigger fish to fry. But with luck somebody will eventually see this github issue, and know just how to solve the chrome-gives-the-wrong-duration-estimates-for-aac-in-m4a problem. Ideally, maybe the JPEG2000 snafu as well ;-)

@EvanHahn-Signal
Copy link
Contributor

We've made a number of changes to voice notes recently and I suspect this is no longer a problem. I'm going to close this issue, but let me know if that's wrong and I can reopen.

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

No branches or pull requests

3 participants