Closed tatokis closed 5 years ago
Thanks for this very detailed report, I'll have a look shortly.
Ok, I think I got it. Wrong implementation of Transfer-Encoding
. A bit annoying to fix but stay tuned for a fix!
Excellent! Thank you for looking into this. Looking forward to testing
Truns out it wasn't so annoying to fix after all. Would you mind testing it? With opam:
git clone --recursive https://github.com/savonet/ocaml-cry.git
cd ocaml-cry
opam pin add .
I was able to only test it shortly, but it seems to be fixed. Thank you!
From what I understand, http/1.1 requires chunked encoding. Shouldn't icecast support it? Could this also be a bug on icecast's end?
It is interesting how ogg wasn't affected though.
Glad to hear. It was a basic protocol logic issue: ocaml-cry
was assuming chunked
transfer encoding for all HTTP/1.1
connections without advertising it to the server.. No idea why ogg wasn't affected, either HTTP/1.0
was still used or perhaps the container is robust enough to get rid of the extra \r\n\r\n
..?
Attempting to stream mpeg4_he_aac_v2 or mp3 to an icecast server built from git master results in the aac stream playing for a few ms before cutting out, and the mp3 stream sounding corrupt.
Thinking it was an icecast issue, I bisected it, and the commit that broke it was https://gitlab.xiph.org/xiph/icecast-server/commit/6d0e4e6fc96d4de94b0f48d0e0f50a8dcc58f4db
Looking through the icecast source, it's obvious that the string is only used for responses, and icecast itself doesn't seem to change its behaviour depending on it, as can be seen here: https://gitlab.xiph.org/search?utf8=%E2%9C%93&search=http_version&group_id=&project_id=2&search_code=true&repository_ref=master
Forcing HTTP/1.0 only for 200 OK and leaving everything else at 1.1 seems to work around this issue:
Using wireshark to monitor the HTTP headers between 1.0 and 1.1 does not reveal anything obvious. Only difference is icecast's response being version 1.1 instead of 1.0.
Example liquidsoap request:
Example icecast response:
I'm not yet entirely sure this is a liquidsoap bug, but other streaming software has no trouble streaming to icecast builds that return HTTP/1.1.
It is worth noting that ogg vorbis streams continue to function and do not seem to be affected by this change.
This was tested against both liquidsoap 1.3.7 and the latest head from master, and both builds exhibit the same issue. I used the latest cry release for both, as there don't seem to be any code changes past its latest release.
This issue can be easily reproduced with an icecast build from master and liquidsoap with
%mp3(bitrate=96)