Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
19419ac
proof of concept jamstreamer first implementation (thanks to hselasky)
dingodoppelt Dec 30, 2020
02cddea
refactor, get rid of unneeded stuff. add comments
dingodoppelt Dec 30, 2020
f097f3d
integrate better into the jamulus server
dingodoppelt Dec 30, 2020
f5f829a
fix for mono-in
dingodoppelt Dec 30, 2020
a17ec71
get rid of float2short conversion and mix on an int16_t vector
dingodoppelt Dec 30, 2020
884fd20
make the streaming and stream destination a commandline option
dingodoppelt Jan 2, 2021
1f34458
handle jamulus server stoped and started signals
dingodoppelt Jan 4, 2021
082dca5
fix build2
dingodoppelt Feb 8, 2021
909a6b0
fix build win32
dingodoppelt Feb 8, 2021
cc48ad5
refactor, get rid of redundant code (thanks to npostavs)
dingodoppelt Feb 10, 2021
2fef426
Make CJamStreamer::process signal & slot pass vector by const ref
npostavs Feb 10, 2021
7f5a61d
Merge pull request #2 from npostavs/streamer2.np.changes
dingodoppelt Feb 11, 2021
8829d1a
Use qproc
npostavs Feb 11, 2021
e3001c5
make qproc into a pointer
npostavs Feb 12, 2021
2374af5
add some error checking
npostavs Feb 12, 2021
e8e2692
adjust logging
npostavs Feb 12, 2021
7b38912
streamer: Add logging of ffmpeg output
hoffie Feb 12, 2021
dc0a184
streamer: Fix ffmpeg invocation
hoffie Feb 12, 2021
b8a8297
add help text
dingodoppelt Feb 13, 2021
c28e784
Merge pull request #3 from hoffie/fix-streamer2
dingodoppelt Feb 13, 2021
04eec1c
initialize qproc
dingodoppelt Feb 17, 2021
2981836
remove comment
dingodoppelt Feb 17, 2021
10400f5
remove comment
dingodoppelt Feb 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Jamulus.pro
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ HEADERS += src/buffer.h \
src/recorder/jamrecorder.h \
src/recorder/creaperproject.h \
src/recorder/cwavestream.h \
src/signalhandler.h
src/signalhandler.h \
src/streamer/jamstreamer.h

HEADERS_GUI = src/audiomixerboard.h \
src/chatdlg.h \
Expand Down Expand Up @@ -512,7 +513,8 @@ SOURCES += src/buffer.cpp \
src/util.cpp \
src/recorder/jamrecorder.cpp \
src/recorder/creaperproject.cpp \
src/recorder/cwavestream.cpp
src/recorder/cwavestream.cpp \
src/streamer/jamstreamer.cpp

SOURCES_GUI = src/audiomixerboard.cpp \
src/chatdlg.cpp \
Expand Down
20 changes: 20 additions & 0 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ int main ( int argc, char** argv )
QString strHTMLStatusFileName = "";
QString strLoggingFileName = "";
QString strRecordingDirName = "";
QString strStreamDest = "";
QString strCentralServer = "";
QString strServerInfo = "";
QString strServerPublicIP = "";
Expand Down Expand Up @@ -369,6 +370,22 @@ int main ( int argc, char** argv )
}


// Stream destination ---------------------------------------------------------
if ( GetStringArgument ( argc,
argv,
i,
"--streamto", // no short form
"--streamto",
strArgument ) )
{
strStreamDest = strArgument;
qInfo() << qUtf8Printable( QString( "- stream destination: %1" )
.arg( strStreamDest ) );
CommandLineOptions << "--streamto";
Comment thread
dingodoppelt marked this conversation as resolved.
continue;
}


// Disable recording on startup ----------------------------------------
if ( GetFlagArgument ( argv,
i,
Expand Down Expand Up @@ -715,6 +732,7 @@ int main ( int argc, char** argv )
strServerListFilter,
strWelcomeMessage,
strRecordingDirName,
strStreamDest,
bDisconnectAllClientsOnQuit,
bUseDoubleSystemFrameSize,
bUseMultithreading,
Expand Down Expand Up @@ -833,6 +851,8 @@ QString UsageArguments ( char **argv )
" --serverpublicip specify your public IP address when\n"
" running a slave and your own central server\n"
" behind the same NAT\n"
" --streamto pass ffmpeg output arguments to stream a stereo mix\n"
" from the server (see \"ffmpeg -h\" for reference)\n"
"\nClient only:\n"
" -M, --mutestream starts the application in muted state\n"
" --mutemyown mute me in my personal mix (headless only)\n"
Expand Down
60 changes: 59 additions & 1 deletion src/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ CServer::CServer ( const int iNewMaxNumChan,
const QString& strServerPublicIP,
const QString& strNewWelcomeMessage,
const QString& strRecordingDirName,
const QString& strStreamDest,
const bool bNDisconnectAllClientsOnQuit,
const bool bNUseDoubleSystemFrameSize,
const bool bNUseMultithreading,
Expand Down Expand Up @@ -405,7 +406,21 @@ CServer::CServer ( const int iNewMaxNumChan,
// that jam recorder needs the frame size which is given to the jam
// recorder in the SetRecordingDir() function)
SetRecordingDir ( strRecordingDirName );

#ifndef _WIN32
// enable jam streaming
if ( !strStreamDest.isEmpty() )
{
bStream = true;
QThread* pthJamStreamer = new QThread;
streamer::CJamStreamer* pJamStreamer = new streamer::CJamStreamer();
pJamStreamer->Init( strStreamDest );
pJamStreamer->moveToThread(pthJamStreamer);
QObject::connect( this, &CServer::Started, pJamStreamer, &streamer::CJamStreamer::OnStarted );
QObject::connect( this, &CServer::Stopped, pJamStreamer, &streamer::CJamStreamer::OnStopped );
QObject::connect( this, &CServer::StreamFrame, pJamStreamer, &streamer::CJamStreamer::process );
pthJamStreamer->start();
}
#endif
// enable all channels (for the server all channel must be enabled the
// entire life time of the software)
for ( i = 0; i < iMaxNumChannels; i++ )
Expand Down Expand Up @@ -863,6 +878,11 @@ static CTimingMeas JitterMeas ( 1000, "test2.dat" ); JitterMeas.Measure(); // TE
vecNumAudioChannels,
vecvecsData,
vecChannelLevels );
#ifndef _WIN32
if ( bStream == true ) {
MixStream ( iNumClients );
}
#endif

for ( int iChanCnt = 0; iChanCnt < iNumClients; iChanCnt++ )
{
Expand Down Expand Up @@ -1331,6 +1351,44 @@ opus_custom_encoder_ctl ( pCurOpusEncoder, OPUS_SET_BITRATE ( CalcBitRateBitsPer
Q_UNUSED ( iUnused )
}

/// @brief Mix the audio data from all clients and send the mix to the jamstreamer
void CServer::MixStream ( const int iNumClients )
{
int i, j, k;
CVector<int16_t>& vecsSendData = vecvecsSendData[0]; // use reference for faster access

// init intermediate processing vector with zeros since we mix all channels on that vector
vecsSendData.Reset ( 0 );

// Stereo target channel -----------------------------------------------
for ( j = 0; j < iNumClients; j++ )
{
// get a reference to the audio data of the current client
const CVector<int16_t>& vecsData = vecvecsData[j];

if ( vecNumAudioChannels[j] == 1 )
{
// mono: copy same mono data in both out stereo audio channels
for ( i = 0, k = 0; i < iServerFrameSizeSamples; i++, k += 2 )
{
// left/right channel
vecsSendData[k] += vecsData[i];
vecsSendData[k + 1] += vecsData[i];
}
}
else
{
// stereo
for ( i = 0; i < ( 2 * iServerFrameSizeSamples ); i++ )
{
vecsSendData[i] += vecsData[i];
}
}
}

emit StreamFrame ( iServerFrameSizeSamples, vecsSendData );
}

CVector<CChannelInfo> CServer::CreateChannelList()
{
CVector<CChannelInfo> vecChanInfo ( 0 );
Expand Down
9 changes: 9 additions & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "serverlogging.h"
#include "serverlist.h"
#include "recorder/jamcontroller.h"
#include "streamer/jamstreamer.h"

/* Definitions ****************************************************************/
// no valid channel number
Expand Down Expand Up @@ -179,6 +180,7 @@ class CServer :
const QString& strServerPublicIP,
const QString& strNewWelcomeMessage,
const QString& strRecordingDirName,
const QString& strStreamDest,
const bool bNDisconnectAllClientsOnQuit,
const bool bNUseDoubleSystemFrameSize,
const bool bNUseMultithreading,
Expand Down Expand Up @@ -314,6 +316,8 @@ class CServer :
void MixEncodeTransmitData ( const int iChanCnt,
const int iNumClients );

void MixStream ( const int iNumClients );

virtual void customEvent ( QEvent* pEvent );

// if server mode is normal or double system frame size
Expand Down Expand Up @@ -391,6 +395,9 @@ class CServer :
recorder::CJamController JamController;
bool bDisableRecording;

// jam streamer
bool bStream = false;

// GUI settings
bool bAutoRunMinimized;

Expand All @@ -412,6 +419,8 @@ class CServer :
const int iNumAudChan,
const CVector<int16_t> vecsData );

void StreamFrame ( const int iServerFrameSizeSamples, const CVector<int16_t>& data );

void CLVersionAndOSReceived ( CHostAddress InetAddr,
COSUtil::EOpSystemType eOSType,
QString strVersion );
Expand Down
72 changes: 72 additions & 0 deletions src/streamer/jamstreamer.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
#ifndef _WIN32
#include "jamstreamer.h"

using namespace streamer;

CJamStreamer::CJamStreamer() : qproc ( NULL ) {}

void CJamStreamer::process( int iServerFrameSizeSamples, const CVector<int16_t>& data ) {
qproc->write ( reinterpret_cast<const char*> (&data[0]), sizeof (int16_t) * ( 2 * iServerFrameSizeSamples ) );
}

void CJamStreamer::Init( const QString strStreamDest ) {
this->strStreamDest = strStreamDest;
}

void CJamStreamer::OnStarted() {
if ( !qproc )
{
qproc = new QProcess;
QObject::connect ( qproc, &QProcess::errorOccurred, this, &CJamStreamer::onError );
QObject::connect ( qproc, QOverload<int, QProcess::ExitStatus>::of( &QProcess::finished ), this, &CJamStreamer::onFinished );
qproc->setStandardOutputFile ( QProcess::nullDevice() );
}
QStringList argumentList = { "-loglevel", "error",
"-y", "-f", "s16le",
"-ar", "48000", "-ac", "2",
"-i", "-" };
argumentList += strStreamDest.split( QRegExp("\\s+") );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should use QProcess::splitCommand for this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should use QProcess::splitCommand for this?

Absolutely! I didn't know about that. What I did just fixed the immediate issue (--streamto "-f mp3 foo.mp3") but now that you mention it, it would probably break when attempting to escape things to ffmpeg (e.g. --streamto "-f mp3 '/tmp/path with spaces'") or something. So my proposal makes things better, but is still not complete. I'm in favor of using splitCommand.

The only problem I see is that this function is only available as of Qt 5.15. As far as I can see, Jamulus is supposed to support earlier versions (5.9?). Therefore, a compromise might be a version check which uses splitCommand if available and falls back to the basic regexp for older versions.

Are you (@npostavs) going to continue with this? Or is @dingodoppelt?
I honestly only looked at this whole PR due to the potential security impact and I only continued investigating after the troubles with QProcess seemed stuck a bit.

Personally, I currently have no need for this feature at all so I would prefer if one of you could continue. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoffie
I'd really love to, but I obviously lack the necessary skills (I'm still overly happy that my first implementation did what it did but that's about what I can contribute since you guys are accomplished programmers and I still don't know what I'm doing here ;)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't realize it 5.15 specific.

Therefore, a compromise might be a version check which uses splitCommand if available and falls back to the basic regexp for older versions.

The compromise sounds like a recipe for confusion to me. How necessary are the multiple arguments? E.g., -f mp3 is redundant if you already give a .mp3 filename anyway, as far as I've understood.

       -f fmt (input/output)
           Force input or output file format. The format is normally auto
           detected for input files and guessed from the file extension for
           output files, so this option is not needed in most cases.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g., -f mp3 is redundant if you already give a .mp3 filename anyway, as far as I've understood.

Yes, my example was hypothetical. No idea why I added that. The help text talks about "ffmpeg arguments", but not sure if or in what circumstances this would be needed. If the requirement could be dropped, this would be the simplest fix. @dingodoppelt?

Copy link
Copy Markdown
Contributor Author

@dingodoppelt dingodoppelt Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoffie
I use it to stream to icecast and that won't work unless the format is specified. Since ffmpeg takes a lot of arguments (even multiple outputs are possible) this is very much needed so you get the full functionality that ffmpeg offers.
The recording to mp3 was just for test purposes so you wouldn't have to set up an icecast server to test the code. I wasn't aware, that the -f option is redundant for I haven't used it to record into a file (I use the awesome jamrecorder for that purpose)

EDIT: This works fine, for example, and represents my usecase:
--streamto "-codec:a libvorbis -qscale:a 3 -content_type 'audio/ogg' -vn -f ogg icecast://..."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how we could move this forward.

The compromise sounds like a recipe for confusion to me.

I generally agree. However, we are talking about edge cases I think (e.g. filenames/urls with spaces). It seems like the half-broken hack I've implemented works for @dingodoppelt's use case. If we can improve that code to be less half-broken on modern Qt versions, I think that's a benefit.

So I'd still vote for the compromise as I don't see any better alternatives?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the other possibilities would be for --streamto to take multiple arguments, a la find -exec ... \;. Or just require a single --streamto argument which is a shell script that calls ffmpeg?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other possibilities would be for --streamto to take multiple arguments, a la find -exec ... ;

Not sure I'm following. Do you mean repeated --streamto arguments which would be concatenated to an argument list by Jamulus? This would delegate the escaping/parsing part to the shell, so we would get it for free. However, usability-wise it does not sound optimal to me.

Or just require a single --streamto argument which is a shell script that calls ffmpeg?

Jamulus-wise, this would be the simplest form. @dingodoppelt Would that work for you as well?
It would have additional advantage that non-ffmpeg use cases could be enabled by this feature as well. It would have the downside that the input details (sample rate, etc.) would have to be specified in such a script.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only problem I see is that this function is only available as of Qt 5.15. As far as I can see, Jamulus is supposed to support earlier versions (5.9?).

I just noticed that the Qt docs seem to say the split functions are only available as of Qt 5.14, so it might be trouble as well?

https://doc.qt.io/qt-5/qstring.html#split-6

// Note that program name is also repeated as first argument
qproc->start ( "ffmpeg", argumentList );
}

void CJamStreamer::onError(QProcess::ProcessError error)
{
QString errDesc;
switch (error) {
case QProcess::FailedToStart:
errDesc = "failed to start";
break;
case QProcess::Crashed:
errDesc = "crashed";
break;
case QProcess::Timedout:
errDesc = "timed out";
break;
case QProcess::WriteError:
errDesc = "write error";
break;
case QProcess::ReadError:
errDesc = "read error";
break;
case QProcess::UnknownError:
errDesc = "unknown error";
break;
default:
errDesc = "UNKNOWN unknown error";
break;
}
qWarning() << "QProcess Error: " << errDesc << "\n";
}

void CJamStreamer::onFinished( int exitCode, QProcess::ExitStatus exitStatus )
{
Q_UNUSED ( exitStatus );
QByteArray stderr = qproc->readAllStandardError ();
qInfo() << "ffmpeg exited with exitCode" << exitCode << ", stderr:" << stderr;
}

void CJamStreamer::OnStopped() {
qproc->closeWriteChannel ();
}
#endif
28 changes: 28 additions & 0 deletions src/streamer/jamstreamer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#ifndef _WIN32
#include <QObject>
#include <QProcess>
#include "../util.h"

namespace streamer {

class CJamStreamer : public QObject {
Q_OBJECT

public:
CJamStreamer();
void Init( const QString strStreamDest );

public slots:
void process( int iServerFrameSizeSamples, const CVector<int16_t>& data );
void OnStarted();
void OnStopped();
private slots:
void onError(QProcess::ProcessError error);
void onFinished( int exitCode, QProcess::ExitStatus exitStatus );

private:
QString strStreamDest; // stream destination to pass to ffmpeg as output part of arguments
QProcess* qproc; // ffmpeg subprocess
};
}
#endif