forkineye / ESPixelStick

Firmware for the ESPixelStick
http://forkineye.com/
528 stars 169 forks source link

Playing a local fseq file does not loop #678

Closed jlange6648 closed 8 months ago

jlange6648 commented 8 months ago

--------- Instructions -------- Please provide answers directly below each section. --------- Instructions ---------

ESPixelStick Firmware Version Main branch as of: commit 43f361d9373b035b77db92195174b76f29b46332 (HEAD -> main, origin/main, origin/HEAD) Merge: 3a559c22 00adb33f Author: Shelby Merrick sporadic@forkineye.com Date: Tue Oct 3 15:03:56 2023 -0400 Merge pull request #672 from MartinMueller2003/main Fixed a small memory leak

Hardware Version D1_mini

Binary release or compiled yourself? either

Operating System (and version) n/a

Web Browser (and version) n/a

Access Point n/a

Describe the bug If using the FPP remote local file input, with the file on the SD card, it will play once, but not loop.

I tracked the issue down to c_InputFPPRemote::StartPlayingLocalFile(). The file to play gets passed in as a String reference to FileBeingPlayed. The first thing StartPlayingLocalFile does, is call StopPlaying(). The first time through, PlayingFile() returns false, so StopPlaying does nothing, and the file plays as expected.

However, on the second time through the loop, PlayingFile() returns true, resulting in StopPlaying to clear out the value of FileBeingPlayed, resulting in the Filename now being blank when the flow gets back to StartPlayingLocalFile.

I was able to fix the issue by passing the filename to StartPlayingLocalFile by value instead of by reference, but I think the root issue is that after the file finished playing, PlayingFile() still returns true. Even if PlayingFile() properly returns false, the fact that the filename value is cleared out from under the call to StartPlayingLocalFile is bad.

Simple fix, that doesn't change the behavior of PlayingFile():

diff --git a/ESPixelStick/src/input/InputFPPRemote.cpp b/ESPixelStick/src/input/InputFPPRemote.cpp
index 2a8323e1..f7c7ee1b 100644
--- a/ESPixelStick/src/input/InputFPPRemote.cpp
+++ b/ESPixelStick/src/input/InputFPPRemote.cpp
@@ -343,7 +343,7 @@ void c_InputFPPRemote::StartPlaying (String& FileName)
 } // StartPlaying

 //-----------------------------------------------------------------------------
-void c_InputFPPRemote::StartPlayingLocalFile (String& FileName)
+void c_InputFPPRemote::StartPlayingLocalFile (String FileName)
 {
     // DEBUG_START;

diff --git a/ESPixelStick/src/input/InputFPPRemote.h b/ESPixelStick/src/input/InputFPPRemote.h
index 849af389..737ed034 100644
--- a/ESPixelStick/src/input/InputFPPRemote.h
+++ b/ESPixelStick/src/input/InputFPPRemote.h
@@ -57,7 +57,7 @@ private:

     void validateConfiguration ();
     void StartPlaying (String & FileName);
-    void StartPlayingLocalFile (String & FileName);
+    void StartPlayingLocalFile (String FileName);
     void StartPlayingRemoteFile (String & FileName);
     void StopPlaying ();
     bool PlayingFile ();
kkarnisky commented 8 months ago

I'm also looking for a fix on this issue. Jeff, thanks for posting the quick fix. Looking forward to a full solution.

onewithhammer commented 8 months ago

I confirm the same issue on a DIG-QUAD with standard module and AE+ module.

MartinMueller2003 commented 8 months ago

Sigh. No good deed etc... I will fix this shortly. It got changed when I was looking for an issue causing flashes in the input stream.

MartinMueller2003 commented 8 months ago

Please try the dist found here and let me know if your issue is resolved: https://drive.google.com/drive/folders/1bGrkXSMoGjWtX5iWspPFRogM2G6c1VXu?usp=sharing

kkarnisky commented 8 months ago

I just tested the attached dist. Seems to work great. Thanks!KeithOn Oct 29, 2023, at 4:52 PM, Martin Mueller @.***> wrote: Please try the dist found here and let me know if your issue is resolved: https://drive.google.com/drive/folders/1bGrkXSMoGjWtX5iWspPFRogM2G6c1VXu?usp=sharing

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

MartinMueller2003 commented 8 months ago

PR: Remote play updates #680