Fix #802: Now honor maxBitrate parameter in Subsonic API

api/funkwhale_api/music/models.py

@@ -853,23 +853,35 @@ class Upload(models.Model):
     def listen_url(self):
         return self.track.listen_url + "?upload={}".format(self.uuid)
 
-    def get_transcoded_version(self, format):
+    def get_transcoded_version(self, format, max_bitrate=None):
-        mimetype = utils.EXTENSION_TO_MIMETYPE[format]
+        if format:
-        existing_versions = list(self.versions.filter(mimetype=mimetype))
+            mimetype = utils.EXTENSION_TO_MIMETYPE[format]
+        else:
+            mimetype = self.mimetype or "audio/mpeg"
+            format = utils.MIMETYPE_TO_EXTENSION[mimetype]
+
+        existing_versions = self.versions.filter(mimetype=mimetype)
+        if max_bitrate is not None:
+            # we don't want to transcode if a 320kbps version is available
+            # and we're requestiong 300kbps
+            acceptable_max_bitrate = max_bitrate * 1.2
+            acceptable_min_bitrate = max_bitrate * 0.8
+            existing_versions = existing_versions.filter(
+                bitrate__gte=acceptable_min_bitrate, bitrate__lte=acceptable_max_bitrate
+            ).order_by("-bitrate")
         if existing_versions:
             # we found an existing version, no need to transcode again
             return existing_versions[0]
 
-        return self.create_transcoded_version(mimetype, format)
+        return self.create_transcoded_version(mimetype, format, bitrate=max_bitrate)
 
     @transaction.atomic
-    def create_transcoded_version(self, mimetype, format):
+    def create_transcoded_version(self, mimetype, format, bitrate):
         # we create the version with an empty file, then
         # we'll write to it
         f = ContentFile(b"")
-        version = self.versions.create(
+        bitrate = min(bitrate or 320000, self.bitrate or 320000)
-            mimetype=mimetype, bitrate=self.bitrate or 128000, size=0
+        version = self.versions.create(mimetype=mimetype, bitrate=bitrate, size=0)
-        )
         # we keep the same name, but we update the extension
         new_name = os.path.splitext(os.path.basename(self.audio_file.name))[
             0
@@ -879,6 +891,7 @@ class Upload(models.Model):
             audio=self.get_audio_segment(),
             output=version.audio_file,
             output_format=utils.MIMETYPE_TO_EXTENSION[mimetype],
+            bitrate=str(bitrate),
         )
         version.size = version.audio_file.size
         version.save(update_fields=["size"])

api/funkwhale_api/music/views.py

@@ -273,25 +273,35 @@ def get_file_path(audio_file):
         return path.encode("utf-8")
 
 
-def should_transcode(upload, format):
+def should_transcode(upload, format, max_bitrate=None):
     if not preferences.get("music__transcoding_enabled"):
         return False
+    format_need_transcoding = True
+    bitrate_need_transcoding = True
     if format is None:
-        return False
+        format_need_transcoding = False
-    if format not in utils.EXTENSION_TO_MIMETYPE:
+    elif format not in utils.EXTENSION_TO_MIMETYPE:
         # format should match supported formats
-        return False
+        format_need_transcoding = False
-    if upload.mimetype is None:
+    elif upload.mimetype is None:
         # upload should have a mimetype, otherwise we cannot transcode
-        return False
+        format_need_transcoding = False
-    if upload.mimetype == utils.EXTENSION_TO_MIMETYPE[format]:
+    elif upload.mimetype == utils.EXTENSION_TO_MIMETYPE[format]:
         # requested format sould be different than upload mimetype, otherwise
         # there is no need to transcode
-        return False
+        format_need_transcoding = False
-    return True
+
+    if max_bitrate is None:
+        bitrate_need_transcoding = False
+    elif not upload.bitrate:
+        bitrate_need_transcoding = False
+    elif upload.bitrate <= max_bitrate:
+        bitrate_need_transcoding = False
+
+    return format_need_transcoding or bitrate_need_transcoding
 
 
-def handle_serve(upload, user, format=None):
+def handle_serve(upload, user, format=None, max_bitrate=None):
     f = upload
     # we update the accessed_date
     now = timezone.now()
@@ -328,8 +338,10 @@ def handle_serve(upload, user, format=None):
         file_path = get_file_path(f.source.replace("file://", "", 1))
     mt = f.mimetype
 
-    if should_transcode(f, format):
+    if should_transcode(f, format, max_bitrate=max_bitrate):
-        transcoded_version = upload.get_transcoded_version(format)
+        transcoded_version = upload.get_transcoded_version(
+            format, max_bitrate=max_bitrate
+        )
         transcoded_version.accessed_date = now
         transcoded_version.save(update_fields=["accessed_date"])
         f = transcoded_version
@@ -377,7 +389,17 @@ class ListenViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
             return Response(status=404)
 
         format = request.GET.get("to")
-        return handle_serve(upload, user=request.user, format=format)
+        max_bitrate = request.GET.get("max_bitrate")
+        try:
+            max_bitrate = min(max(int(max_bitrate), 0), 320) or None
+        except (TypeError, ValueError):
+            max_bitrate = None
+
+        if max_bitrate:
+            max_bitrate = max_bitrate * 1000
+        return handle_serve(
+            upload, user=request.user, format=format, max_bitrate=max_bitrate
+        )
 
 
 class UploadViewSet(

api/funkwhale_api/subsonic/views.py

@@ -250,7 +250,18 @@ class SubsonicViewSet(viewsets.GenericViewSet):
         format = data.get("format", "raw")
         if format == "raw":
             format = None
-        return music_views.handle_serve(upload=upload, user=request.user, format=format)
+
+        max_bitrate = data.get("maxBitRate")
+        try:
+            max_bitrate = min(max(int(max_bitrate), 0), 320) or None
+        except (TypeError, ValueError):
+            max_bitrate = None
+
+        if max_bitrate:
+            max_bitrate = max_bitrate * 1000
+        return music_views.handle_serve(
+            upload=upload, user=request.user, format=format, max_bitrate=max_bitrate
+        )
 
     @action(detail=False, methods=["get", "post"], url_name="star", url_path="star")
     @find_object(music_models.Track.objects.all())

api/tests/music/test_views.py

@@ -344,7 +344,7 @@ def test_listen_explicit_file(factories, logged_in_api_client, mocker):
 
     assert response.status_code == 200
     mocked_serve.assert_called_once_with(
-        upload2, user=logged_in_api_client.user, format=None
+        upload2, user=logged_in_api_client.user, format=None, max_bitrate=None
     )
 
 
@@ -367,6 +367,22 @@ def test_should_transcode(mimetype, format, expected, factories):
     assert views.should_transcode(upload, format) is expected
 
 
+@pytest.mark.parametrize(
+    "bitrate,max_bitrate,expected",
+    [
+        # already in acceptable bitrate
+        (192000, 320000, False),
+        # No max bitrate specified
+        (192000, None, False),
+        # requested max below available
+        (192000, 128000, True),
+    ],
+)
+def test_should_transcode_bitrate(bitrate, max_bitrate, expected, factories):
+    upload = models.Upload(mimetype="audio/mpeg", bitrate=bitrate)
+    assert views.should_transcode(upload, "mp3", max_bitrate=max_bitrate) is expected
+
+
 @pytest.mark.parametrize("value", [True, False])
 def test_should_transcode_according_to_preference(value, preferences, factories):
     upload = models.Upload(mimetype="audio/ogg")
@@ -404,7 +420,35 @@ def test_listen_transcode(factories, now, logged_in_api_client, mocker):
     assert response.status_code == 200
 
     handle_serve.assert_called_once_with(
-        upload, user=logged_in_api_client.user, format="mp3"
+        upload, user=logged_in_api_client.user, format="mp3", max_bitrate=None
+    )
+
+
+@pytest.mark.parametrize(
+    "max_bitrate, expected",
+    [
+        ("", None),
+        ("", None),
+        ("-1", None),
+        ("128", 128000),
+        ("320", 320000),
+        ("460", 320000),
+    ],
+)
+def test_listen_transcode_bitrate(
+    max_bitrate, expected, factories, now, logged_in_api_client, mocker
+):
+    upload = factories["music.Upload"](
+        import_status="finished", library__actor__user=logged_in_api_client.user
+    )
+    url = reverse("api:v1:listen-detail", kwargs={"uuid": upload.track.uuid})
+    handle_serve = mocker.spy(views, "handle_serve")
+    response = logged_in_api_client.get(url, {"max_bitrate": max_bitrate})
+
+    assert response.status_code == 200
+
+    handle_serve.assert_called_once_with(
+        upload, user=logged_in_api_client.user, format=None, max_bitrate=expected
     )
 
 
@@ -430,7 +474,7 @@ def test_listen_transcode_in_place(
     assert response.status_code == 200
 
     handle_serve.assert_called_once_with(
-        upload, user=logged_in_api_client.user, format="mp3"
+        upload, user=logged_in_api_client.user, format="mp3", max_bitrate=None
     )
 
 

api/tests/subsonic/test_views.py

@@ -226,7 +226,7 @@ def test_stream(f, db, logged_in_api_client, factories, mocker, queryset_equal_q
     response = logged_in_api_client.get(url, {"f": f, "id": upload.track.pk})
 
     mocked_serve.assert_called_once_with(
-        upload=upload, user=logged_in_api_client.user, format=None
+        upload=upload, user=logged_in_api_client.user, format=None, max_bitrate=None
     )
     assert response.status_code == 200
     playable_by.assert_called_once_with(music_models.Track.objects.all(), None)
@@ -242,7 +242,26 @@ def test_stream_format(format, expected, logged_in_api_client, factories, mocker
     response = logged_in_api_client.get(url, {"id": upload.track.pk, "format": format})
 
     mocked_serve.assert_called_once_with(
-        upload=upload, user=logged_in_api_client.user, format=expected
+        upload=upload, user=logged_in_api_client.user, format=expected, max_bitrate=None
+    )
+    assert response.status_code == 200
+
+
+@pytest.mark.parametrize(
+    "max_bitrate,expected", [(0, None), (192, 192000), (2000, 320000)]
+)
+def test_stream_bitrate(max_bitrate, expected, logged_in_api_client, factories, mocker):
+    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}
+    )
+
+    mocked_serve.assert_called_once_with(
+        upload=upload, user=logged_in_api_client.user, format=None, max_bitrate=expected
     )
     assert response.status_code == 200
 

changes/changelog.d/802.enhancement

@@ -0,0 +1 @@
+Now honor maxBitrate parameter in Subsonic API (#802)