Commit f1088fa8 authored by Philippe Gorley's avatar Philippe Gorley Committed by Hugo Lefeuvre

recorder: refactor and simplify

1. Adds constness to streams in MediaRecorder; the latter shouldn't change
stream parameters.
2. addStream is called from recordData to simplify workflow; users need
only check that MediaRecorder is recording.
3. Fixes regression where multiple records couldn't happen in the same
call.
4. Fixes AudioSender not sending starting timestamp to help mix audio.
5. MediaStream::name is now used as key for the streams_ map, so it must
be constant, filter names are no longer hardcoded in the recorder.
6. fromPeer parameter is determined by checking if MediaStream::name
contains the string "remote".
7. Adds copy constructor to MediaStream.
8. Renames incrementStreams to incrementExpectedStreams to better reflect
what it does.
9. Removes keyframe triggers when starting a video record; we are
recording raw frames, so it doesn't matter if they're keyframes or not.
10. MediaRecorder supports more than 2 audio streams, but not video.
11. Move preview to very bottom right corner.
12. Fix potential Unchecked Return Value to NULL Pointer Dereference in
recordData() (CWE-690).

Gitlab: #39

Change-Id: Id2fae4e9bb2072994c065e4843cc3ad832f89efc
parent 9d36c29e
......@@ -75,7 +75,7 @@ class AudioSender {
std::unique_ptr<Resampler> resampler_;
std::weak_ptr<MediaRecorder> recorder_;
bool recordingStarted_ = false;
uint64_t sent_samples = 0;
AudioBuffer micData_;
......@@ -208,22 +208,15 @@ AudioSender::process()
buffer.reset();
AVFrame* frame = buffer.toAVFrame();
auto ms = MediaStream("audio", buffer.getFormat());
auto ms = MediaStream("a:local", buffer.getFormat());
frame->pts = getNextTimestamp(sent_samples, ms.sampleRate, static_cast<rational<int64_t>>(ms.timeBase));
ms.firstTimestamp = frame->pts;
sent_samples += frame->nb_samples;
{
auto rec = recorder_.lock();
if (rec && !recordingStarted_ && rec->addStream(false, false, ms) >= 0) {
recordingStarted_ = true;
}
if (rec && recordingStarted_) {
rec->recordData(frame, false, false);
} else {
recordingStarted_ = false;
recorder_ = std::weak_ptr<MediaRecorder>();
}
if (rec && rec->isRecording())
rec->recordData(frame, ms);
}
if (audioEncoder_->encodeAudio(frame) < 0)
......@@ -245,11 +238,8 @@ AudioSender::getLastSeqValue()
void
AudioSender::initRecorder(std::shared_ptr<MediaRecorder>& rec)
{
recordingStarted_ = false;
recorder_ = rec;
if (auto r = recorder_.lock()) {
r->incrementStreams(1);
}
rec->incrementExpectedStreams(1);
}
class AudioReceiveThread
......@@ -277,7 +267,6 @@ class AudioReceiveThread
bool decodeFrame();
std::weak_ptr<MediaRecorder> recorder_;
bool recordingStarted_{false};
/*-----------------------------------------------------------------*/
/* These variables should be used in thread (i.e. process()) only! */
......@@ -369,16 +358,8 @@ AudioReceiveThread::process()
case MediaDecoder::Status::FrameFinished:
{
auto rec = recorder_.lock();
if (rec && !recordingStarted_ && rec->addStream(false, true, audioDecoder_->getStream()) >= 0) {
recordingStarted_ = true;
}
if (rec && recordingStarted_) {
rec->recordData(decodedFrame.pointer(), false, true);
} else {
recordingStarted_ = false;
recorder_ = std::weak_ptr<MediaRecorder>();
}
if (rec && rec->isRecording())
rec->recordData(decodedFrame.pointer(), audioDecoder_->getStream("a:remote"));
}
audioDecoder_->writeToRingBuffer(decodedFrame, *ringbuffer_,
mainBuffFormat);
......@@ -449,8 +430,8 @@ AudioReceiveThread::startLoop()
void
AudioReceiveThread::initRecorder(std::shared_ptr<MediaRecorder>& rec)
{
rec->incrementStreams(1);
recorder_ = rec;
rec->incrementExpectedStreams(1);
}
AudioRtpSession::AudioRtpSession(const std::string& id)
......
......@@ -505,9 +505,9 @@ MediaDecoder::correctPixFmt(int input_pix_fmt) {
}
MediaStream
MediaDecoder::getStream() const
MediaDecoder::getStream(std::string name) const
{
auto ms = MediaStream("", decoderCtx_, lastTimestamp_);
auto ms = MediaStream(name, decoderCtx_, lastTimestamp_);
#ifdef RING_ACCEL
if (decoderCtx_->codec_type == AVMEDIA_TYPE_VIDEO && enableAccel_ && !accel_.name.empty())
ms.format = AV_PIX_FMT_NV12; // TODO option me!
......
......@@ -100,7 +100,7 @@ class MediaDecoder {
void enableAccel(bool enableAccel);
#endif
MediaStream getStream() const;
MediaStream getStream(std::string name = "") const;
private:
NON_COPYABLE(MediaDecoder);
......
This diff is collapsed.
......@@ -60,7 +60,7 @@ class MediaRecorder {
// adjust nb of streams before recording
// used to know when all streams are set up
void incrementStreams(int n);
void incrementExpectedStreams(int n);
bool isRecording() const;
......@@ -70,17 +70,17 @@ class MediaRecorder {
void stopRecording();
int addStream(bool isVideo, bool fromPeer, MediaStream ms);
int recordData(AVFrame* frame, bool isVideo, bool fromPeer);
int recordData(AVFrame* frame, const MediaStream& ms);
private:
NON_COPYABLE(MediaRecorder);
int addStream(const MediaStream& ms);
int initRecord();
MediaStream setupVideoOutput();
std::string buildVideoFilter();
std::string buildVideoFilter(const std::vector<MediaStream>& peers, const MediaStream& local) const;
MediaStream setupAudioOutput();
std::string buildAudioFilter(const std::vector<MediaStream>& peers, const MediaStream& local) const;
void emptyFilterGraph();
int sendToEncoder(AVFrame* frame, int streamIdx);
int flush();
......@@ -92,8 +92,7 @@ class MediaRecorder {
std::mutex mutex_; // protect against concurrent file writes
// isVideo is first key, fromPeer is second
std::map<bool, std::map<bool, MediaStream>> streams_;
std::map<std::string, const MediaStream> streams_;
std::tm startTime_;
std::string title_;
......
......@@ -77,8 +77,7 @@ struct MediaStream {
MediaStream(std::string name, AVCodecContext* c)
: MediaStream(name, c, 0)
{
}
{}
MediaStream(std::string name, AVCodecContext* c, int64_t firstTimestamp)
: name(name)
......
......@@ -232,19 +232,10 @@ bool VideoInput::captureFrame()
return static_cast<bool>(decoder_);
case MediaDecoder::Status::FrameFinished:
if (auto rec = recorder_.lock()) {
if (!recordingStarted_) {
if (rec->addStream(true, false, decoder_->getStream()) >= 0) {
recordingStarted_ = true;
} else {
recorder_ = std::weak_ptr<MediaRecorder>();
}
}
if (recordingStarted_)
rec->recordData(frame.pointer(), true, false);
} else {
recordingStarted_ = false;
recorder_ = std::weak_ptr<MediaRecorder>();
{
auto rec = recorder_.lock();
if (rec && rec->isRecording())
rec->recordData(frame.pointer(), decoder_->getStream("v:local"));
}
publishFrame();
return true;
......@@ -607,8 +598,8 @@ VideoInput::foundDecOpts(const DeviceParams& params)
void
VideoInput::initRecorder(std::shared_ptr<MediaRecorder>& rec)
{
rec->incrementStreams(1);
recorder_ = rec;
rec->incrementExpectedStreams(1);
}
}} // namespace ring::video
......@@ -145,7 +145,6 @@ private:
#endif
std::weak_ptr<MediaRecorder> recorder_;
bool recordingStarted_{false};
};
}} // namespace ring::video
......
......@@ -184,19 +184,10 @@ bool VideoReceiveThread::decodeFrame()
switch (ret) {
case MediaDecoder::Status::FrameFinished:
if (auto rec = recorder_.lock()) {
if (!recordingStarted_) {
if (rec->addStream(true, true, videoDecoder_->getStream()) >= 0) {
recordingStarted_ = true;
} else {
recorder_ = std::weak_ptr<MediaRecorder>();
}
}
if (recordingStarted_)
rec->recordData(frame.pointer(), true, true);
} else {
recordingStarted_ = false;
recorder_ = std::weak_ptr<MediaRecorder>();
{
auto rec = recorder_.lock();
if (rec && rec->isRecording())
rec->recordData(frame.pointer(), videoDecoder_->getStream("v:remote"));
}
publishFrame();
return true;
......@@ -269,8 +260,8 @@ VideoReceiveThread::triggerKeyFrameRequest()
void
VideoReceiveThread::initRecorder(std::shared_ptr<ring::MediaRecorder>& rec)
{
rec->incrementStreams(1);
recorder_ = rec;
rec->incrementExpectedStreams(1);
}
}} // namespace ring::video
......@@ -90,7 +90,6 @@ private:
static int readFunction(void *opaque, uint8_t *buf, int buf_size);
std::weak_ptr<MediaRecorder> recorder_;
bool recordingStarted_{false};
ThreadLoop loop_;
......
......@@ -568,15 +568,10 @@ void
VideoRtpSession::initRecorder(std::shared_ptr<MediaRecorder>& rec)
{
// video recording needs to start with keyframes
const constexpr int keyframes = 3;
if (receiveThread_)
receiveThread_->initRecorder(rec);
if (auto vidInput = std::static_pointer_cast<VideoInput>(videoLocal_))
vidInput->initRecorder(rec);
for (int i = 0; i < keyframes; ++i)
if (receiveThread_)
receiveThread_->triggerKeyFrameRequest();
// TODO trigger keyframes for local video
}
}} // namespace ring::video
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment