search

ripose-jp / Memento

An mpv-based video player for studying Japanese
https://ripose-jp.github.io/Memento/
GNU General Public License v2.0
434 stars 21 forks source link

src/audio/audioplayer: do not use open() on QNetworkReply #224

Closed SpaghettiBorgar closed 2 weeks ago

SpaghettiBorgar commented 2 weeks ago

For some reason, calling open() on the network reply for an audio clip, clears out the buffer so you can no longer read data from it anymore, unless it is internally handled as a zero-copy buffer, which is not public.

During testing, trying to play an audio clip for a word from assets.japanesepod101.com results in an empty temp file being written. Calling bytesAvailable() before open() gives a valid number, but after the open() it becomes 0 and no data can be read. However, trying to retrieve audio for non-existing words (With the voiced response "The audio for this clip is currently not available...") works fine and bytesAvailable() will show about 50KB even after open() is called.

After a lot of debugging, it turns out that in the latter case, the data is stored in the private downloadZerocopyBuffer of the QNetworkReplyHttpImplPrivate class, while it was stored in a "normal" buffer in the other cases, perhaps as an optimization further down the network stack that only happens with larger payloads (Most other audio clips seem to be only 2-5KB in size).

This honestly seems like a Qt bug to me, but I couldn't find any report and struggled to set up a minimal example to make one myself. I also couldn't find any example of people using open() on a QNetworkReply object. It looks like it's not something necessary to do, so removing it fixes the issue.

I tested this on Arch Linux with kernel 6.9.3-zen and Qt 6.7.1, but remember audio clips already being broken for some while before that.

ripose-jp commented 2 weeks ago

Good find. I hadn't noticed the bug since that code actually works with older versions of Qt. You're probably right about there being some sort of regression in more recent versions. I don't know why I called open on that code originally. It's possible that I had seen some example code do it, but I honestly don't remember.

remember audio clips already being broken for some while before that.

JapanesePod had moved their API to a new TLD and had the old API redirect. Qt by default doesn't follow redirects for security reasons, so that's probably why they were broken initially.