Skip to content

Commit

Permalink
Convert to std::array and std::string. (libmythtv/recorders)
Browse files Browse the repository at this point in the history
  • Loading branch information
@linuxdude42
linuxdude42 committed Aug 28, 2020
1 parent 15d7643 commit 818eced
Show file tree
Hide file tree
Showing 22 changed files with 260 additions and 250 deletions.
14 changes: 6 additions & 8 deletions mythtv/libs/libmythtv/recorders/DeviceReadBuffer.cpp
View file
Expand Up @@ -179,12 +179,11 @@ void DeviceReadBuffer::SetPaused(bool val)
// The WakePoll code is copied from MythSocketThread::WakeReadyReadThread()
void DeviceReadBuffer::WakePoll(void) const
{
char buf[1];
buf[0] = '0';
std::string buf(1,'\0');
ssize_t wret = 0;
while (isRunning() && (wret <= 0) && (m_wakePipe[1] >= 0))
{
wret = ::write(m_wakePipe[1], &buf, 1);
wret = ::write(m_wakePipe[1], buf.data(), buf.size());
if ((wret < 0) && (EAGAIN != errno) && (EINTR != errno))
{
LOG(VB_GENERAL, LOG_ERR, LOC + "WakePoll failed.");
Expand Down Expand Up @@ -423,8 +422,7 @@ bool DeviceReadBuffer::Poll(void) const
timer.start();

int poll_cnt = 1;
struct pollfd polls[2];
memset(polls, 0, sizeof(polls));
std::array<struct pollfd,2> polls {};

polls[0].fd = m_streamFd;
polls[0].events = POLLIN | POLLPRI;
Expand All @@ -451,7 +449,7 @@ bool DeviceReadBuffer::Poll(void) const
// subtract a bit to allow processing time.
timeout = max((int)m_maxPollWait - timer.elapsed() - 15, 10);

int ret = poll(polls, poll_cnt, timeout);
int ret = poll(polls.data(), poll_cnt, timeout);

if (polls[0].revents & POLLHUP)
{
Expand Down Expand Up @@ -506,9 +504,9 @@ bool DeviceReadBuffer::Poll(void) const
// Clear out any pending pipe reads
if ((poll_cnt > 1) && (polls[1].revents & POLLIN))
{
char dummy[128];
std::array<char,128> dummy {};
int cnt = (m_wakePipeFlags[0] & O_NONBLOCK) ? 128 : 1;
::read(m_wakePipe[0], dummy, cnt);
::read(m_wakePipe[0], dummy.data(), cnt);
}

if (m_pollTimeoutIsError && (timer.elapsed() >= (int)m_maxPollWait))
Expand Down
23 changes: 11 additions & 12 deletions mythtv/libs/libmythtv/recorders/ExternalStreamHandler.cpp
View file
Expand Up @@ -73,12 +73,11 @@ ExternIO::~ExternIO(void)
bool ExternIO::Ready(int fd, int timeout, const QString & what)
{
#if !defined( USING_MINGW ) && !defined( _MSC_VER )
struct pollfd m_poll[2];
memset(m_poll, 0, sizeof(m_poll));
std::array<struct pollfd,2> m_poll {};

m_poll[0].fd = fd;
m_poll[0].events = POLLIN | POLLPRI;
int ret = poll(m_poll, 1, timeout);
int ret = poll(m_poll.data(), 1, timeout);

if (m_poll[0].revents & POLLHUP)
{
Expand Down Expand Up @@ -177,9 +176,9 @@ QString ExternIO::GetStatus(int timeout)
int waitfor = m_status.atEnd() ? timeout : 0;
if (Ready(m_appErr, waitfor, "status"))
{
char buffer[2048];
int len = read(m_appErr, buffer, 2048);
m_status << QString::fromLatin1(buffer, len);
std::array<char,2048> buffer {};
int len = read(m_appErr, buffer.data(), buffer.size());
m_status << QString::fromLatin1(buffer.data(), len);
}

if (m_status.atEnd())
Expand Down Expand Up @@ -320,23 +319,23 @@ void ExternIO::Fork(void)

LOG(VB_RECORD, LOG_INFO, QString("ExternIO::Fork '%1'").arg(full_command));

int in[2] = {-1, -1};
int out[2] = {-1, -1};
int err[2] = {-1, -1};
std::array<int,2> in = {-1, -1};
std::array<int,2> out = {-1, -1};
std::array<int,2> err = {-1, -1};

if (pipe(in) < 0)
if (pipe(in.data()) < 0)
{
m_error = "pipe(in) failed: " + ENO;
return;
}
if (pipe(out) < 0)
if (pipe(out.data()) < 0)
{
m_error = "pipe(out) failed: " + ENO;
close(in[0]);
close(in[1]);
return;
}
if (pipe(err) < 0)
if (pipe(err.data()) < 0)
{
m_error = "pipe(err) failed: " + ENO;
close(in[0]);
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythtv/recorders/HLS/HLSReader.cpp
View file
Expand Up @@ -945,7 +945,7 @@ int HLSReader::DownloadSegmentData(MythSingleDownload& downloader,
/* If the segment is encrypted, decode it */
if (segment.HasKeyPath())
{
if (!hls->DecodeData(downloader, hls->IVLoaded() ? hls->AESIV() : nullptr,
if (!hls->DecodeData(downloader, hls->IVLoaded() ? hls->AESIV() : QByteArray(),
segment.KeyPath(),
buffer, segment.Sequence()))
return 0;
Expand Down
16 changes: 7 additions & 9 deletions mythtv/libs/libmythtv/recorders/HLS/HLSStream.cpp
View file
Expand Up @@ -68,7 +68,7 @@ bool HLSRecStream::DownloadKey(MythSingleDownload& downloader,
}

bool HLSRecStream::DecodeData(MythSingleDownload& downloader,
const uint8_t *IV, const QString& keypath,
const QByteArray& IV, const QString& keypath,
QByteArray& data, int64_t sequence)
{
AESKeyMap::iterator Ikey;
Expand All @@ -88,9 +88,9 @@ bool HLSRecStream::DecodeData(MythSingleDownload& downloader,

/* Decrypt data using AES-128 */
int aeslen = data.size() & ~0xf;
unsigned char iv[AES_BLOCK_SIZE];
std::array<uint8_t,AES_BLOCK_SIZE> iv {};
char *decrypted_data = new char[data.size()];
if (IV == nullptr)
if (IV.isEmpty())
{
/*
* If the EXT-X-KEY tag does not have the IV attribute,
Expand All @@ -100,21 +100,19 @@ bool HLSRecStream::DecodeData(MythSingleDownload& downloader,
* the sequence number SHALL be placed in a 16-octet
* buffer and padded (on the left) with zeros.
*/
memset(iv, 0, AES_BLOCK_SIZE);
iv[15] = sequence & 0xff;
iv[14] = (sequence >> 8) & 0xff;
iv[13] = (sequence >> 16) & 0xff;
iv[12] = (sequence >> 24) & 0xff;
}
else
{
memcpy(iv, IV, sizeof(iv));
std::copy(IV.cbegin(), IV.cend(), iv.data());
}
AES_cbc_encrypt((unsigned char*)data.constData(),
(unsigned char*)decrypted_data, aeslen,
*Ikey, iv, AES_DECRYPT);
memcpy(decrypted_data + aeslen, data.constData() + aeslen,
data.size() - aeslen);
*Ikey, iv.data(), AES_DECRYPT);
std::copy(data.cbegin() + aeslen, data.cend(), decrypted_data + aeslen);

// remove the PKCS#7 padding from the buffer
// NOLINTNEXTLINE(bugprone-signed-char-misuse)
Expand Down Expand Up @@ -180,7 +178,7 @@ bool HLSRecStream::SetAESIV(QString line)
int padding = std::max(0, AES_BLOCK_SIZE - (line.size() - 2));
QByteArray ba = QByteArray(padding, 0x0);
ba.append(QByteArray::fromHex(QByteArray(line.toLatin1().constData() + 2)));
memcpy(m_aesIV, ba.constData(), ba.size());
m_aesIV = ba;
m_ivLoaded = true;
return true;
}
Expand Down
6 changes: 3 additions & 3 deletions mythtv/libs/libmythtv/recorders/HLS/HLSStream.h
View file
Expand Up @@ -64,12 +64,12 @@ class HLSRecStream
bool DownloadKey(MythSingleDownload& downloader,
const QString& keypath, AES_KEY* aeskey);
bool DecodeData(MythSingleDownload& downloader,
const uint8_t *IV, const QString& keypath,
const QByteArray& IV, const QString& keypath,
QByteArray& data, int64_t sequence);
bool SetAESIV(QString line);
bool IVLoaded(void) const { return m_ivLoaded; }

uint8_t *AESIV(void) { return m_aesIV; }
QByteArray AESIV(void) { return m_aesIV; }
void SetKeyPath(const QString& x) { m_keypath = x; }
#endif // USING_LIBCRYPTO

Expand Down Expand Up @@ -98,7 +98,7 @@ class HLSRecStream
private:
QString m_keypath; // URL path of the encrypted key
bool m_ivLoaded {false};
uint8_t m_aesIV[AES_BLOCK_SIZE] {0};// IV used when decypher the block
QByteArray m_aesIV {AES_BLOCK_SIZE,0};// IV used when decypher the block
AESKeyMap m_aesKeys; // AES-128 keys by path
#endif // USING_LIBCRYPTO
};
Expand Down
65 changes: 27 additions & 38 deletions mythtv/libs/libmythtv/recorders/NuppelVideoRecorder.cpp
View file
Expand Up @@ -1365,8 +1365,8 @@ void NuppelVideoRecorder::DoV4L2(void)

numbuffers = vrbuf.count;

unsigned char *buffers[MAX_VIDEO_BUFFERS];
int bufferlen[MAX_VIDEO_BUFFERS];
std::array<uint8_t*,MAX_VIDEO_BUFFERS> buffers {};
std::array<int,MAX_VIDEO_BUFFERS> bufferlen {};

for (uint i = 0; i < numbuffers; i++)
{
Expand Down Expand Up @@ -1875,11 +1875,11 @@ void NuppelVideoRecorder::SetNewVideoParams(double newaspect)
void NuppelVideoRecorder::WriteFileHeader(void)
{
struct rtfileheader fileheader {};
static constexpr char kFinfo[12] = "MythTVVideo";
static constexpr char kVers[5] = "0.07";
static const std::string kFinfo { "MythTVVideo" };
static const std::string kVers { "0.07" };

memcpy(fileheader.finfo, kFinfo, sizeof(fileheader.finfo));
memcpy(fileheader.version, kVers, sizeof(fileheader.version));
std::copy(kFinfo.cbegin(), kFinfo.cend(), fileheader.finfo);
std::copy(kVers.cbegin(), kVers.cend(), fileheader.version);
fileheader.width = m_wOut;
fileheader.height = (int)(m_hOut * m_heightMultiplier);
fileheader.desiredwidth = 0;
Expand Down Expand Up @@ -1926,16 +1926,15 @@ void NuppelVideoRecorder::WriteHeader(void)
}
else
{
static unsigned long int s_tbls[128];
static std::array<uint32_t,128> s_tbls {};

frameheader.comptype = 'R'; // compressor data for RTjpeg
frameheader.packetlength = sizeof(s_tbls);
frameheader.packetlength = s_tbls.size() * sizeof(uint32_t);

// compression configuration header
WriteFrameheader(&frameheader);

memset(s_tbls, 0, sizeof(s_tbls));
m_ringBuffer->Write(s_tbls, sizeof(s_tbls));
m_ringBuffer->Write(s_tbls.data(), s_tbls.size() * sizeof(uint32_t));
}

memset(&frameheader, 0, sizeof(frameheader));
Expand Down Expand Up @@ -2315,8 +2314,8 @@ void NuppelVideoRecorder::FormatTT(struct VBIData *vbidata)
unsigned char *outpos = m_textBuffer[act]->buffer;
*outpos = 0;
struct teletextsubtitle st {};
unsigned char linebuf[VT_WIDTH + 1];
unsigned char *linebufpos = linebuf;
std::vector<uint8_t> linebuf {};
linebuf.reserve(VT_WIDTH + 1);

for (int y = 0; y < VT_HEIGHT; y++)
{
Expand Down Expand Up @@ -2419,11 +2418,9 @@ void NuppelVideoRecorder::FormatTT(struct VBIData *vbidata)
st.dbl = dbl;
st.fg = fg;
st.bg = bg;
linebufpos = linebuf;
*linebufpos = 0;
linebuf.clear();
}
*linebufpos++ = c;
*linebufpos = 0;
linebuf.push_back(c);
visible = 1;
}

Expand All @@ -2432,23 +2429,16 @@ void NuppelVideoRecorder::FormatTT(struct VBIData *vbidata)
}
if (visible)
{
st.len = linebufpos - linebuf + 1;;
int max = 200;
st.len = linebuf.size() + 1;
int maxlen = 200;
int bufsize = ((outpos - m_textBuffer[act]->buffer + 1) + st.len);
if (bufsize > max)
if (bufsize > maxlen)
break;
memcpy(outpos, &st, sizeof(st));
outpos += sizeof(st);
if (st.len < 42)
{
memcpy(outpos, linebuf, st.len);
outpos += st.len;
}
else
{
memcpy(outpos, linebuf, 41);
outpos += 41;
}
int count = max(st.len, static_cast<uint8_t>(41));
std::copy(linebuf.cbegin(), linebuf.cbegin() + count, outpos);
outpos += count;
*outpos = 0;
}
}
Expand Down Expand Up @@ -2673,7 +2663,7 @@ void NuppelVideoRecorder::WriteVideo(VideoFrame *frame, bool skipsync,
int raw = 0;
int compressthis = m_compression;
// cppcheck-suppress variableScope
uint8_t *planes[3] = {
std::array<uint8_t*,3> planes {
frame->buf + frame->offsets[0],
frame->buf + frame->offsets[1],
frame->buf + frame->offsets[2] };
Expand Down Expand Up @@ -2784,7 +2774,7 @@ void NuppelVideoRecorder::WriteVideo(VideoFrame *frame, bool skipsync,
{
if (wantkeyframe)
m_rtjc->SetNextKey();
tmp = m_rtjc->Compress(m_strm, planes);
tmp = m_rtjc->Compress(m_strm, planes.data());
}
else
tmp = frame->size;
Expand All @@ -2796,11 +2786,11 @@ void NuppelVideoRecorder::WriteVideo(VideoFrame *frame, bool skipsync,
if (raw)
{
r = lzo1x_1_compress(frame->buf, frame->size,
m_out, &out_len, wrkmem);
m_out.data(), &out_len, wrkmem);
}
else
{
r = lzo1x_1_compress((unsigned char *)m_strm, tmp, m_out,
r = lzo1x_1_compress((unsigned char *)m_strm, tmp, m_out.data(),
&out_len, wrkmem);
}
if (r != LZO_E_OK)
Expand Down Expand Up @@ -2866,7 +2856,7 @@ void NuppelVideoRecorder::WriteVideo(VideoFrame *frame, bool skipsync,
frameheader.comptype = '3'; // raw YUV420 with lzo
frameheader.packetlength = out_len;
WriteFrameheader(&frameheader);
m_ringBuffer->Write(m_out, out_len);
m_ringBuffer->Write(m_out.data(), out_len);
}

if (m_framesWritten == 0)
Expand Down Expand Up @@ -2940,7 +2930,7 @@ void NuppelVideoRecorder::WriteAudio(unsigned char *buf, int fnum, int timecode)

if (m_compressAudio)
{
char mp3gapless[7200];
std::array<uint8_t,7200> mp3gapless {};
int compressedsize = 0;
int gaplesssize = 0;
int lameret = 0;
Expand Down Expand Up @@ -2974,8 +2964,7 @@ void NuppelVideoRecorder::WriteAudio(unsigned char *buf, int fnum, int timecode)
}
compressedsize = lameret;

lameret = lame_encode_flush_nogap(m_gf, (unsigned char *)mp3gapless,
7200);
lameret = lame_encode_flush_nogap(m_gf, mp3gapless.data(), mp3gapless.size());
if (lameret < 0)
{
LOG(VB_GENERAL, LOG_ERR, LOC +
Expand All @@ -2993,7 +2982,7 @@ void NuppelVideoRecorder::WriteAudio(unsigned char *buf, int fnum, int timecode)
{
WriteFrameheader(&frameheader);
m_ringBuffer->Write(m_mp3Buf, compressedsize);
m_ringBuffer->Write(mp3gapless, gaplesssize);
m_ringBuffer->Write(mp3gapless.data(), gaplesssize);
}
m_audioBytes += m_audioBufferSize;
}
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythtv/recorders/NuppelVideoRecorder.h
View file
Expand Up @@ -190,7 +190,7 @@ class MTV_PUBLIC NuppelVideoRecorder : public V4LRecorder, public CC608Input
RTjpeg *m_rtjc {nullptr};

#define OUT_LEN (1024*1024 + 1024*1024 / 64 + 16 + 3)
lzo_byte m_out[OUT_LEN] {};
array<lzo_byte,OUT_LEN> m_out {};
#define HEAP_ALLOC(var,size) \
long __LZO_MMODEL var [ ((size) + (sizeof(long) - 1)) / sizeof(long) ]
HEAP_ALLOC(wrkmem, LZO1X_1_MEM_COMPRESS) {};
Expand Down

5 comments on commit 818eced

@kmdewaal
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit does cause conditional access to fail with this message:
2020-09-11 16:14:01.764336 E ERROR: CAM: Data pointer null
Restoring dvbcam.cpp, dvbcam.h, dvbci.cpp and dvbci.h to the state before this commit does restore correct behavior in current master.

@kmdewaal
Copy link
Contributor

Choose a reason for hiding this comment

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

Another issue is that the "htonl" macro is replaced by little-endian only encoding. Is this needed for c++17 compliance, is it suggested by clang-tidy or is the goal to remove big-endian support from mythtv?

@linuxdude42
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This commit does cause conditional access to fail with this message:
2020-09-11 16:14:01.764336 E ERROR: CAM: Data pointer null
Restoring dvbcam.cpp, dvbcam.h, dvbci.cpp and dvbci.h to the state before this commit does restore correct behavior in current master.

Does it work if you change the test on dvbci.cpp:831 from "if (!Data)" to "if (Length > 0 && !Data)"?

@linuxdude42
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another issue is that the "htonl" macro is replaced by little-endian only encoding.

The new code should work for either big or little endian devices, converting them to network encoding. Arithmetic operations are endian agnostic.

The htonl macro can't be used in a uint8_t constructor because it returns a 32-bit value. The code could be modified to resize the vector by four and then use the previous "*(int *)p = htonl(ResourceId)" line, but then you end up writing the bytes twice, once with zeros and then once with the parts of ResourceId.

Is this needed for c++17 compliance, is it suggested by clang-tidy or is the goal to remove big-endian support from mythtv?

No, no, and of course not.

@kmdewaal
Copy link
Contributor

Choose a reason for hiding this comment

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

My recommendation is to revert the changes in dvbci.cpp, dvbci.h, dvbcam.cpp and dvbcam.h. As I understand it the new code does not fix any outstanding problem and the old code works fine so there is no urgent need to rewrite it. Also, the way I understand the MythTV Coding Standards, it is recommended to "Make large style changes in a separate patch from functional changes" and your changes to the CI handler look like a functional change to me.

Please sign in to comment.