diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index b88a8daea..9e0268504 100644 --- a/api/funkwhale_api/music/models.py +++ b/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): - mimetype = utils.EXTENSION_TO_MIMETYPE[format] - existing_versions = list(self.versions.filter(mimetype=mimetype)) + def get_transcoded_version(self, format, max_bitrate=None): + if format: + 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( - mimetype=mimetype, bitrate=self.bitrate or 128000, size=0 - ) + bitrate = min(bitrate or 320000, self.bitrate or 320000) + 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"]) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 2f0e67cb9..eeaa80124 100644 --- a/api/funkwhale_api/music/views.py +++ b/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 - if format not in utils.EXTENSION_TO_MIMETYPE: + format_need_transcoding = False + elif format not in utils.EXTENSION_TO_MIMETYPE: # format should match supported formats - return False - if upload.mimetype is None: + format_need_transcoding = False + elif upload.mimetype is None: # upload should have a mimetype, otherwise we cannot transcode - return False - if upload.mimetype == utils.EXTENSION_TO_MIMETYPE[format]: + format_need_transcoding = False + 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 - return True + format_need_transcoding = False + + 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): - transcoded_version = upload.get_transcoded_version(format) + if should_transcode(f, format, max_bitrate=max_bitrate): + 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( diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index c40ed8ced..31bd00407 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/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()) diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 2c3a61c05..ed38afcda 100644 --- a/api/tests/music/test_views.py +++ b/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 ) diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index e75bfc2a0..ec61b25fa 100644 --- a/api/tests/subsonic/test_views.py +++ b/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 diff --git a/changes/changelog.d/802.enhancement b/changes/changelog.d/802.enhancement new file mode 100644 index 000000000..25d4f3e5d --- /dev/null +++ b/changes/changelog.d/802.enhancement @@ -0,0 +1 @@ +Now honor maxBitrate parameter in Subsonic API (#802)