Merge branch '802-maxBitrate' into 'develop'
Fix #802: Now honor maxBitrate parameter in Subsonic API Closes #802 See merge request funkwhale/funkwhale!721
This commit is contained in:
commit
ae390e5c1c
|
@ -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"])
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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())
|
||||
|
|
|
@ -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
|
||||
)
|
||||
|
||||
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -0,0 +1 @@
|
|||
Now honor maxBitrate parameter in Subsonic API (#802)
|
Loading…
Reference in New Issue