Merge pull request #416 from kagemomiji/issue223-change-play-count-logic

#223 Fix incrementPlayCount method to include player parameter
kagemomiji · Apr 30, 2024 · 04cceb3 · 04cceb3
2 parents 33ac370 + ba696db
commit 04cceb3
Show file tree

Hide file tree

Showing 12 changed files with 66 additions and 16 deletions.
diff --git a/airsonic-main/src/main/java/org/airsonic/player/ajax/PlayQueueWSController.java b/airsonic-main/src/main/java/org/airsonic/player/ajax/PlayQueueWSController.java
@@ -48,6 +48,15 @@ public void stop(@DestinationVariable("playerId") Integer playerId, SimpMessageH
         playQueueService.stop(player);
     }
 
+    @MessageMapping("/endMedia")
+    public void endMedia(@DestinationVariable("playerId") Integer playerId, Integer mediaFileId, SimpMessageHeaderAccessor headers) throws Exception {
+        if (mediaFileId == null) {
+            return;
+        }
+        Player player = getPlayer(playerId, headers);
+        playQueueService.endMedia(player, mediaFileId);
+    }
+
     @MessageMapping("/toggleStartStop")
     public void toggleStartStop(@DestinationVariable("playerId") Integer playerId, SimpMessageHeaderAccessor headers) throws Exception {
         Player player = getPlayer(playerId, headers);

diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/HLSController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/HLSController.java
@@ -332,7 +332,7 @@ public ResponseEntity<Resource> handleSegmentRequest(Authentication auth,
         BiConsumer<InputStream, TransferStatus> inputStreamInit = (i, s) -> {
             LOG.info("{}: {} listening to {}", player.getIpAddress(), player.getUsername(), FileUtil.getShortPath(mediaFile.getRelativePath()));
             if (segmentIndex == 0)
-                this.mediaFileService.incrementPlayCount(mediaFile);
+                this.mediaFileService.incrementPlayCount(player, mediaFile);
         };
 
         Resource resource = new MonitoredResource(new PathResource(segmentFile),

diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java
@@ -213,13 +213,17 @@ public ResponseEntity<Resource> handleRequest(Authentication authentication,
 
         Consumer<MediaFile> fileStartListener = mediaFile -> {
             LOG.info("{}: {} listening to {} in folder {}", player.getIpAddress(), player.getUsername(), FileUtil.getShortPath(mediaFile.getRelativePath()), mediaFile.getFolder().getId());
-            mediaFileService.incrementPlayCount(mediaFile);
             scrobble(mediaFile, player, false);
             status.setMediaFile(mediaFile);
             statusService.addActiveLocalPlay(
                     new PlayStatus(status.getId(), mediaFile, player, status.getMillisSinceLastUpdate()));
         };
-        Consumer<MediaFile> fileEndListener = mediaFile -> {
+        BiConsumer<Integer, MediaFile> fileEndListener = (readCount, mediaFile) -> {
+            if (readCount != null && readCount > 0 && player.getTechnology() != PlayerTechnology.WEB) {
+                // Increment play count if the file was actually played
+                // WEB player increments play count on the client side
+                mediaFileService.incrementPlayCount(player, mediaFile);
+            }
             scrobble(mediaFile, player, true);
             statusService.removeActiveLocalPlay(
                     new PlayStatus(status.getId(), mediaFile, player, status.getMillisSinceLastUpdate()));

diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/SubsonicRESTController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/SubsonicRESTController.java
@@ -1442,7 +1442,7 @@ public void scrobble(HttpServletRequest request, HttpServletResponse response) t
             Instant time = times.length == 0 ? null : Instant.ofEpochMilli(times[i]);
 
             statusService.addRemotePlay(new PlayStatus(UUID.randomUUID(), file, player, time == null ? Instant.now() : time));
-            mediaFileService.incrementPlayCount(file);
+            mediaFileService.incrementPlayCount(player, file);
             audioScrobblerService.register(file, player.getUsername(), submission, time);
         }
 

diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/VideoPlayerController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/VideoPlayerController.java
@@ -122,6 +122,7 @@ protected ModelAndView handleRequestInternal(HttpServletRequest request, HttpSer
         // map.put("bitRates", BIT_RATES);
         map.put("defaultBitRate", streamUrls.getLeft());
         map.put("user", user);
+        map.put("playerId", playerId);
 
         return new ModelAndView("videoPlayer", "model", map);
     }

diff --git a/airsonic-main/src/main/java/org/airsonic/player/io/PlayQueueInputStream.java b/airsonic-main/src/main/java/org/airsonic/player/io/PlayQueueInputStream.java
@@ -5,19 +5,21 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
 
 public class PlayQueueInputStream extends InputStream {
     private final PlayQueue queue;
     private final Consumer<MediaFile> fileStartListener;
-    private final Consumer<MediaFile> fileEndListener;
+    private final BiConsumer<Integer, MediaFile> fileEndListener;
     private final Function<MediaFile, InputStream> streamGenerator;
     private InputStream currentStream;
     private MediaFile currentFile;
+    private Integer readCount = 0;
 
     public PlayQueueInputStream(PlayQueue queue, Consumer<MediaFile> fileStartListener,
-            Consumer<MediaFile> fileEndListener, Function<MediaFile, InputStream> streamGenerator) {
+            BiConsumer<Integer, MediaFile> fileEndListener, Function<MediaFile, InputStream> streamGenerator) {
         this.queue = queue;
         this.fileStartListener = fileStartListener;
         this.fileEndListener = fileEndListener;
@@ -71,6 +73,8 @@ private void prepare() throws IOException {
             currentFile = file;
             fileStartListener.accept(currentFile);
             currentStream = streamGenerator.apply(currentFile);
+        } else {
+            readCount++;
         }
     }
 
@@ -80,9 +84,10 @@ public void closeStream() throws IOException {
             currentStream = null;
         }
         if (currentFile != null) {
-            fileEndListener.accept(currentFile);
+            fileEndListener.accept(readCount, currentFile);
             currentFile = null;
         }
+        readCount = 0;
     }
 
     @Override

diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/JukeboxLegacySubsonicService.java b/airsonic-main/src/main/java/org/airsonic/player/service/JukeboxLegacySubsonicService.java
@@ -174,7 +174,7 @@ private void onSongStart(MediaFile file) {
         status = statusService.createStreamStatus(player);
         status.setMediaFile(file);
         status.addBytesTransferred(file.getFileSize());
-        mediaFileService.incrementPlayCount(file);
+        mediaFileService.incrementPlayCount(status.getPlayer(), file);
         playStatus = new PlayStatus(status.getId(), file, status.getPlayer(), status.getMillisSinceLastUpdate());
         statusService.addActiveLocalPlay(playStatus);
         scrobble(file, false);

diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/MediaFileService.java b/airsonic-main/src/main/java/org/airsonic/player/service/MediaFileService.java
@@ -77,6 +77,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.time.Duration;
 import java.time.Instant;
 import java.time.ZoneId;
 import java.time.format.DateTimeFormatter;
@@ -130,6 +131,8 @@ public class MediaFileService {
 
     private final double DURATION_EPSILON = 1e-2;
 
+    private final Map<Integer, Pair<Integer, Instant>> lastPlayed = new ConcurrentHashMap<>();
+
     public MediaFile getMediaFile(String pathName) {
         return getMediaFile(Paths.get(pathName));
     }
@@ -1482,8 +1485,16 @@ public void updateMediaFile(@Nonnull MediaFile mediaFile) {
      * directory and album.
      */
     @Transactional
-    public void incrementPlayCount(MediaFile file) {
+    public void incrementPlayCount(Player player, MediaFile file) {
         Instant now = Instant.now();
+
+        Pair<Integer, Instant> lastPlayedInfo = lastPlayed.computeIfAbsent(player.getId(), k -> Pair.of(file.getId(), now));
+        if (lastPlayedInfo.getLeft() == file.getId()) {
+            Double threshold = Math.max(1.0, file.getDuration() / 2);
+            if (Duration.between(lastPlayedInfo.getRight(), now).getSeconds() < threshold) {
+                return;
+            }
+        }
         file.setLastPlayed(now);
         file.setPlayCount(file.getPlayCount() + 1);
         updateMediaFile(file);
@@ -1501,6 +1512,8 @@ public void incrementPlayCount(MediaFile file) {
                 albumRepository.save(album);
             }
         );
+
+        lastPlayed.put(player.getId(), Pair.of(file.getId(), now));
     }
 
     public List<MediaFileEntry> toMediaFileEntryList(List<MediaFile> files, String username, boolean calculateStarred, boolean calculateFolderAccess,

diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/PlayQueueService.java b/airsonic-main/src/main/java/org/airsonic/player/service/PlayQueueService.java
@@ -25,6 +25,8 @@
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
+import jakarta.annotation.Nonnull;
+
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -86,6 +88,14 @@ public void stop(Player player) {
                 PlayQueue.Status.STOPPED);
     }
 
+    public void endMedia(Player player, @Nonnull Integer mediaFileId) {
+        MediaFile file = mediaFileService.getMediaFile(mediaFileId);
+        if (file == null) {
+            return;
+        }
+        mediaFileService.incrementPlayCount(player, file);
+    }
+
     public void toggleStartStop(Player player) {
         if (player.getPlayQueue().getStatus() == PlayQueue.Status.STOPPED) {
             start(player);

diff --git a/airsonic-main/src/main/resources/templates/playQueue.html b/airsonic-main/src/main/resources/templates/playQueue.html
@@ -664,6 +664,8 @@
 
                 onEnded() {
                     this.setBookmark();
+                    var song = this.songs[this.currentSongIndex];
+                    top.StompClient.send("/app/playqueues/" + this.player.id + "/endMedia", song.id);
                     this.onNext(this.repeatStatus);
                 },
 
@@ -711,7 +713,7 @@
                         castAppID: "4FBFE470",
                         features: ["speed", "chromecast"],
                         defaultSpeed: "1.00",
-                        speeds: ["8.00", "2.00", "1.50", "1.25", "1.00", "0.75", "0.5"],
+                        speeds: ["2.00", "1.50", "1.25", "1.00", "0.75", "0.5"],
                         success(mediaElement, originalNode, instance) {
                             // "hack" html5 renderer and reinitialize speed
                             instance.media.rendererName = "html5";

diff --git a/airsonic-main/src/main/resources/templates/videoPlayer.html b/airsonic-main/src/main/resources/templates/videoPlayer.html
@@ -62,7 +62,8 @@
           videoBookmarkFrequency: /*[(${model.videoBookmarkFrequency})]*/ 30,
           contentType: "[(${model.contentType})]",
           hideShare: /*[(${model.user.shareRole ? 'true': 'false'})]*/ false,
-          hideDownload: /*[(${model.user.downloadRole ? 'true': 'false'})]*/ false
+          hideDownload: /*[(${model.user.downloadRole ? 'true': 'false'})]*/ false,
+          playerId: /*[(${model.playerId})]*/ 0
         }
 
         function setBookmark() {
@@ -74,6 +75,11 @@
                 }
             }
         }
+        function onEnded() {
+            this.setBookmark();
+            top.StompClient.send("/app/playqueues/" + videoModel.playerId + "/endMedia", videoModel.videoId);
+        }
+
 
         function init() {
             $.get(videoModel.remoteCaptionsListUrl, data => {
@@ -95,7 +101,7 @@
                     path: "[(@{/script/mediaelement/renderers/dash.all-4.6.0.min.js})]"
                 },
                 defaultSpeed: "1.00",
-                speeds: ["8.00", "2.00", "1.50", "1.25", "1.00", "0.75", "0.5"],
+                speeds: ["2.00", "1.50", "1.25", "1.00", "0.75", "0.5"],
                 defaultQuality: "[(${model.defaultBitRate})]",
                 videoWidth: "100%",
                 videoHeight: "100%",
@@ -123,7 +129,7 @@
                     instance.setCurrentTime(videoModel.position/1000);
 
                     // Once playback reaches the end, go to the next song, if any.
-                    $(mediaElement).on("ended", () => vpr.setBookmark());
+                    $(mediaElement).on("ended", () => vpr.onEnded());
                     $(mediaElement).on("timeupdate", () => vpr.setBookmark());
                     $(mediaElement).on("seeked", () => vpr.setBookmark());
                     $(mediaElement).on("paused", () => vpr.setBookmark());

diff --git a/...onic-main/src/test/java/org/airsonic/player/service/JukeboxLegacySubsonicServiceTest.java b/...onic-main/src/test/java/org/airsonic/player/service/JukeboxLegacySubsonicServiceTest.java
@@ -132,7 +132,7 @@ public void testUpdateJukeBoxWithPlayingPlayerShouldPlay() throws Exception {
 
             // onSongStart
             verify(statusService).createStreamStatus(mockedPlayer);
-            verify(mediaFileService).incrementPlayCount(mockedMediaFile);
+            verify(mediaFileService).incrementPlayCount(mockedPlayer, mockedMediaFile);
             verify(statusService).addActiveLocalPlay(any());
 
             // scrobble
@@ -254,7 +254,7 @@ public void testUpdateJukeBoxWithPlayingPlayerShouldUpdatePlayer() throws Except
             verify(secondAudioPlayer).setGain(eq(0.75f)); // default gain
             verify(secondAudioPlayer).play();
             verify(statusService).createStreamStatus(secondPlayer);
-            verify(mediaFileService).incrementPlayCount(secondMediaFile);
+            verify(mediaFileService).incrementPlayCount(secondPlayer, secondMediaFile);
             verify(audioScrobblerService).register(eq(secondMediaFile), eq("test"), eq(false), isNull());
         }
 
@@ -292,7 +292,7 @@ public void testStateChangedShouldPlayNext() throws Exception {
             verify(mockedAudioPlayer, times(2)).setGain(eq(0.75f)); // default gain
             verify(mockedAudioPlayer, times(2)).play();
             verify(statusService, times(2)).createStreamStatus(mockedPlayer);
-            verify(mediaFileService).incrementPlayCount(secondMediaFile);
+            verify(mediaFileService).incrementPlayCount(mockedPlayer, secondMediaFile);
             verify(audioScrobblerService).register(eq(secondMediaFile), eq("test"), eq(false), isNull());
 
         }