From bfff79301d4e2e26d1732e390261b109e2d2b23f Mon Sep 17 00:00:00 2001 From: Eliot Berriot <contact@eliotberriot.com> Date: Fri, 28 Jun 2019 09:59:57 +0200 Subject: [PATCH] Fix #867: Added a SUBSONIC_DEFAULT_TRANSCODING_FORMAT env var to support clients that don't provide the format parameter --- api/config/settings/common.py | 3 +++ api/funkwhale_api/subsonic/views.py | 14 ++++++++---- api/tests/subsonic/test_views.py | 35 ++++++++++++++++++++++++----- changes/changelog.d/867.enhancement | 1 + 4 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 changes/changelog.d/867.enhancement diff --git a/api/config/settings/common.py b/api/config/settings/common.py index ab060b448d..654b58b40a 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -658,3 +658,6 @@ RSA_KEY_SIZE = 2048 CREATE_IMAGE_THUMBNAILS = env.bool("CREATE_IMAGE_THUMBNAILS", default=True) # we rotate actor keys at most every two days by default ACTOR_KEY_ROTATION_DELAY = env.int("ACTOR_KEY_ROTATION_DELAY", default=3600 * 48) +SUBSONIC_DEFAULT_TRANSCODING_FORMAT = ( + env("SUBSONIC_DEFAULT_TRANSCODING_FORMAT", default="mp3") or None +) diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 88d7ece7c5..52f37c8704 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -247,10 +247,6 @@ class SubsonicViewSet(viewsets.GenericViewSet): if not upload: return response.Response(status=404) - format = data.get("format", "raw") - if format == "raw": - format = None - max_bitrate = data.get("maxBitRate") try: max_bitrate = min(max(int(max_bitrate), 0), 320) or None @@ -259,6 +255,16 @@ class SubsonicViewSet(viewsets.GenericViewSet): if max_bitrate: max_bitrate = max_bitrate * 1000 + + format = data.get("format", "raw") or None + if max_bitrate and not format: + # specific bitrate requested, but no format specified + # so we use a default one, cf #867. This helps with clients + # that don't send the format parameter, such as DSub. + format = settings.SUBSONIC_DEFAULT_TRANSCODING_FORMAT + elif format == "raw": + format = None + return music_views.handle_serve( upload=upload, user=request.user, diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index 73f968ff47..cde07ac53d 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -261,23 +261,48 @@ def test_stream_format(format, expected, logged_in_api_client, factories, mocker @pytest.mark.parametrize( - "max_bitrate,expected", [(0, None), (192, 192000), (2000, 320000)] + "max_bitrate,format,default_transcoding_format,expected_bitrate,expected_format", + [ + # no max bitrate, no format, so no transcoding should happen + (0, "", "ogg", None, None), + # same using "raw" format + (0, "raw", "ogg", None, None), + # specified bitrate, but no format, so fallback to default transcoding format + (192, "", "ogg", 192000, "ogg"), + # specified bitrate, but over limit + (2000, "", "ogg", 320000, "ogg"), + # specified format, we use that one + (192, "opus", "ogg", 192000, "opus"), + # No default transcoding format set and no format requested + (192, "", None, 192000, None), + ], ) -def test_stream_bitrate(max_bitrate, expected, logged_in_api_client, factories, mocker): +def test_stream_transcode( + max_bitrate, + format, + default_transcoding_format, + expected_bitrate, + expected_format, + logged_in_api_client, + factories, + mocker, + settings, +): + settings.SUBSONIC_DEFAULT_TRANSCODING_FORMAT = default_transcoding_format url = reverse("api:subsonic-stream") mocked_serve = mocker.patch.object( music_views, "handle_serve", return_value=Response() ) upload = factories["music.Upload"](playable=True) response = logged_in_api_client.get( - url, {"id": upload.track.pk, "maxBitRate": max_bitrate} + url, {"id": upload.track.pk, "maxBitRate": max_bitrate, "format": format} ) mocked_serve.assert_called_once_with( upload=upload, user=logged_in_api_client.user, - format=None, - max_bitrate=expected, + format=expected_format, + max_bitrate=expected_bitrate, proxy_media=True, ) assert response.status_code == 200 diff --git a/changes/changelog.d/867.enhancement b/changes/changelog.d/867.enhancement new file mode 100644 index 0000000000..ed889184d9 --- /dev/null +++ b/changes/changelog.d/867.enhancement @@ -0,0 +1 @@ +Added a SUBSONIC_DEFAULT_TRANSCODING_FORMAT env var to support clients that don't provide the format parameter (#867) -- GitLab