Problem sending unsigned char array with QUdpSocket

zcybercomputing · wrote on 17 Oct 2014, 17:15

Hi,

I am trying to port the udp streaming portion of a program to use QUdpSocket. I have to take in packets as:

@unsigned char pkt_buffer[]@

I then append them to a QByteArray in groups of seven packets. I have tested the datagram building code using a dummy packet payload generated by using

@
unsigned char test_char = 'testing this function';
new_datagram.append(test_char);
@

This has properly formed packets in wireshark. When I try using the pktBuffer[] array, I get malformed udp packets. Wireshark doesn't see a destination port for many packets. Some packets display PTS in the destination field.

Right now, i am not really worried about the contents of the packets, as this is a drop in replacement for a previous function built on winsock. That version outputs correct packets, although with performance issues. I think my issue is with dealing with the unsigned char array correctly. Could anyone please instruct me on the proper way to do this?

Each packet is 188 bytes long of type unsigned char.

@
#include "stream.h"

#include "mux_control.h"
/// ========================================================================================================
stream::stream(QHostAddress stream_addr, qint16 stream_port, int kBitRate, QObject parent) :
QObject(parent)
{
udp_streaming_socket = new QUdpSocket(this);
ip_stream_address = stream_addr;
ip_stream_port = stream_port;
// 8 bits per byte, 188 bytes per packet, and 7 packets per datagram.
timer_freq = (8188*7)/(kBitRate); // ms between packets.
packet_timer = new QTimer(this);
connect(packet_timer, SIGNAL(timeout()), this, SLOT(send_udp_packet()));

datagram_buffer.clear();
new_datagram.clear();
packet_timer->start(timer_freq);

// packet_timer->start(1000);
// bits per Datagram, and Bitrate (Bit Rate in Kbps/1000)
// Send datagram to server (D2Mux) after Waiting to "Pace" sending Datagram by three things: "Bitrate" and
// ..."Ticks per sec" (CPU Speed) and "Pkts Per Datagram"

// Wait for time to send datagram... (81887)/3300000 = 0.00319030303... Sec
// (81887)/2874000 = 0.0036631872 Sec
// For Bitrate of 3300 kbps & 7 Pkts per Datagram, Datagram should be sent every .0031903... Sec (approx 3.19 ms or 0.00319 sec)
// For Bitrate 0f 2784 kbps & 7 Pkts per Datagram, Datagram should be sent every .0036632 Sec (approx 3.66 ms or 0.00366 sec)

qDebug("Opened Streaming port");

}

stream::~stream()
{
qDebug("stopping stream");
udp_streaming_socket->close();
}

void stream::make_udp_packet(unsigned char pktBuffer[], int kBitRate, int pktsPerDgram)
{
if (new_datagram.size()<DGRAM_SIZE)
{
new_datagram.append((char*)pktBuffer, 188);
}
else if(new_datagram.size()>=DGRAM_SIZE)
{
datagram_buffer.append(new_datagram);
new_datagram.clear();
}
}

void stream::send_udp_packet()
{
if(datagram_buffer.size()<=1)
{
packet_timer->stop();
}
if(datagram_buffer.size()>=1)
{
udp_streaming_socket->writeDatagram(datagram_buffer.first().data(), datagram_buffer.first().size(),ip_stream_address , ip_stream_port);
datagram_buffer.removeFirst();
}
}

@

SGaist · wrote on 17 Oct 2014, 22:02

Hi and welcome to devnet,

Not a direct answer but there are some things puzzling in your code:

What is datagram_buffer ?

In make_udp_packet, if your new_datagram.size() is over DGRAM_SIZE, you just put the old value in your datagram_buffer, but you don't do anything to keep the pktBuffer, why ?

zcybercomputing · wrote on 17 Oct 2014, 22:34

Ok, so pktBuffer holds MPEG-TS packets. Each datagram needs to hold 7 packets. That is why I have the extra logic. I have tested that logic with other data types, and get decent UDP packets, but for some reason unsigned char arrays like pktBuffer are problematic.

The idea is make_udp_packet() is called each time a packet becomes availible and assembles the 7 packet 188 byte datagram.

make_udp_packet() is called from the timer to send those packets out at an even interval.

The previous version of the code used memmove and some global counters to accomplish this, but I would prefer to use built in parts of Qt if possible. I find most of the Qt stuff is much more readable.

Maybe a header will help.

@
#ifndef STREAM_H
#define STREAM_H

#include "QtCore"
#include <QUdpSocket>
#include <string> // to get string type;
#include <windows.h>
extern int gThreshold;

class stream : public QObject
{

Q_OBJECT

public:
explicit stream(QHostAddress stream_addr, qint16 stream_port,int kBitRate, QObject *parent = 0);
~stream();
QHostAddress ip_stream_address;
qint16 ip_stream_port;
int timer_freq;
#define AD_PORT 7 // Insert Program Input on ASI-1 = 0, ASI-2 = 1, IP-2 = 7, IP-3 = 8, IP-4 = 9
// IP-1 is Reserved for EAS Replacement of Video Streams (Max = 8)
#define AD_PROG 1 // Insert Program

#define ASI_1             0x01                         // 0000 0001  (bit 0)
#define ASI_2             0x02                         // 0000 0010  (bit 1)
#define EAS_DESTINATIONS  ASI_1                        // Or ASI_2
// Number of Source PIDs is "Index 2 of minorChans" array above
#define VID_STREAM_TYPE   0x02
#define VID_PID           0x44  //0x44               // Also used in Fix_VLC.cpp
#define AUD_STREAM_TYPE   0x81
#define AUD_PID           0x45  //0x45               // Also used in Fix_VLC.cpp

#define PKTS_PER_DGRAM    7
#define PKT_SIZE          188                        // 188 Bytes per TS Pkt
#define DGRAM_SIZE        PKTS_PER_DGRAM * PKT_SIZE    // Datagram Size = 7 Transport Stream Packets of 188 bytes each = 1316

// UDP.cpp

// Functions provided...
// openUdp (Init UDP), closeUdp,
// outputPktUdp (MPEG over Ip via Datagram (UDP/RTP) Streaming,
// sendUdpToMiniMux (Send D2Alert Protocol by UDP on Port 0x4541 "EA")

    FILE *udpTestFile;

// Flags and Int Values for Output UDP/RTP Stream
    int  gPktNum      = 0;
    int  gDatagramNum = 0;
    bool gReadBufferFullyWritten = false;
    bool gSendFlag    = true;

// Values for Pacing and Checking Datagram Send Times
    LARGE_INTEGER ticksPerSecond;   // Time Tick Rate determined by CPU speed
    LARGE_INTEGER now;      // Current Time
    LARGE_INTEGER start, stop, oldStop;        // Test Time
    LARGE_INTEGER deltaUdpDatagramTime;  // Time between UDP Sends, based on Bitrate and ticksPerSecond
    LARGE_INTEGER sendNextDatagram;         // Time to Send next Datagram; (lastDgramSent + deltaUdpDatagramTime)
    LARGE_INTEGER lastDgramSent;            // "Scheduled Time" of Last Datagram Sent
    LARGE_INTEGER udpWaitTime;
    LARGE_INTEGER lastNow;
    LARGE_INTEGER late;
    LARGE_INTEGER sendTm;

// Buffers
    unsigned char datagramBuffer[DGRAM_SIZE]; // datagramBuffer set to maximum size for 7 MPEG Pkts
    unsigned char udpSendBuffer[DGRAM_SIZE];

public slots:
void send_udp_packet();
void make_udp_packet(unsigned char pktBuffer[], int kBitRate, int pktsPerDgram);
signals:

private:
QUdpSocket *udp_streaming_socket;
QByteArray stream_datagram;
QTimer *packet_timer;
QByteArray new_datagram;
QList< QByteArray > datagram_buffer;
};

#endif // STREAM_H

@

zcybercomputing · wrote on 17 Oct 2014, 22:46

[quote author="SGaist" date="1413583323"]Hi and welcome to devnet,
In make_udp_packet, if your new_datagram.size() is over DGRAM_SIZE, you just put the old value in your datagram_buffer, but you don't do anything to keep the pktBuffer, why ?[/quote]

new_datagram begins in the initialize as being cleared. Thus, new_datagram.size() <DGRAM_SIZE to start out with. In the second if statement, new_datagram is cleared again. Because the size if pktBuffer is allways 188 bytes, and DGRAM_SIZE is calculated from that, there should not be any condition that causes new_datagram.size() to be larger then DGRAM_SIZE.

Not sure what the best way to post it would be, but the wireshark capture of the packets is weird. Occasionally, a packet will get sent with a proper address destination and sender field parsed by wireshark. Most of the packets either have that field empty, but some have the PMT in the destination field. The PMT is a field in my MPEG-TS packets. The packets comming in via pktBuffer should be valid, because they worked fine using the old winsock code. I didn't like the while loop wait "clocks" in that code though. They had performance inconsistencies.

zcybercomputing · wrote on 19 Oct 2014, 18:59

Any help on this?

 I tried using memmove to assemble the datagram instead of appending, and get the same results.  If I create a qbytearray of size DGRAM_SIZE full of zeros and use that as my datagram, I get a proper address source and destination fields, but when filling it with my packet information derived from unsigned char array packets, the address fields do not show up properly in wireshark for most of the datagrams.  
 I can pick up the stream in VLC, and pieces of the picture decode, but it looks like a stream with a huge amount of packet loss.  My thoughts are that it is not seeing any of the datagrams that lack the correct destination address.  Is this a bug in QUdpSocket, or is it just an issue with my conversion from unsigned char to QByteArray?

 This is a very frustrating issue.

zcybercomputing · wrote on 19 Oct 2014, 22:29

 I have determined that null packets generated in my program pass fine via QUdpSocket, but packets read from a file cause a problem.  I would like to be able to connect the udp streaming code to the rest of the program without rewriting the whole thing, so I am not sure if I really want to change to using the qt file readers.
 Because of this, I created a test program to send out 50 packets from the file, or 50 null packets as a demo.  This demonstrates that somehow the contents of the packet can mess up the structure of the resulting UDP packet.  I think this may constitute a bug.  This happens both when using fread and QFile to input the file.

zip file with the whole project including .pro file. (if using a shadow build, copy the .ts file to your build dir)
http://goo.gl/AJHV4W

mainwindow.h
@
#ifndef MAINWINDOW_H
#define MAINWINDOW_H
#include "stream.h"
#include <QMainWindow>

namespace Ui {
class MainWindow;
}

class MainWindow : public QMainWindow
{
Q_OBJECT

public:
explicit MainWindow(QWidget *parent = 0);
~MainWindow();

private slots:
void on_pushButton_clicked();
void on_pushButton_2_clicked();

void on_pushButton_3_clicked();

private:
Ui::MainWindow *ui;
stream *stream_video;
int packets_read;
#define K_BIT_RATE 2742 // 5000 4400 In kbps //2875
#define VLC_OUTPUT_FILE "vlcOutputFile.ts"
unsigned char pkt[PKT_SIZE];
unsigned char nullPkt[PKT_SIZE];
QHostAddress stream_addr;
qint16 stream_port = 1234;
};
#endif // MAINWINDOW_H

@
mainwindow.cpp
@
#include "mainwindow.h"
#include "ui_mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);
nullPkt[0] = 0x47;
nullPkt[1] = 0x1f;
nullPkt[2] = 0xff;
nullPkt[3] = 0x10;
for (int i=4; i <= 187; i++) {
nullPkt[i] = 0xff; // Fill nullPkt[ ]; 0..187
}
stream_addr.setAddress("239.0.0.225");
stream_video = new stream(stream_addr, stream_port, K_BIT_RATE,this);
}

MainWindow::~MainWindow()
{
delete ui;
}
bool running;
void MainWindow::on_pushButton_clicked()
{

FILE *origFile;
origFile  = fopen &#40;VLC_OUTPUT_FILE, "rb"&#41;;
for(int i=0;i<50*7;i++)
{
packets_read = fread(pkt, PKT_SIZE, 1, origFile);
stream_video->make_udp_packet(pkt,K_BIT_RATE,7);
}

}

void MainWindow::on_pushButton_2_clicked()
{
for(int i=0;i<50*7;i++)
{
stream_video->make_udp_packet(nullPkt,K_BIT_RATE,7);
}

}

void MainWindow::on_pushButton_3_clicked()
{
char packet[PKT_SIZE];

QFile file&#40;VLC_OUTPUT_FILE&#41;;
if (!file.open(QIODevice::ReadOnly))
{
    qDebug("File not opened");
}
else
    for(int i=0;i<50*7;i++)
    {
        qDebug("reading file");
        packets_read  = file.read(packet,PKT_SIZE);
        qDebug("sending packet");
         stream_video->make_udp_packet(reinterpret_cast<unsigned char*>(packet),K_BIT_RATE,7);
        qDebug("here");
    }

}

@

zcybercomputing · wrote on 20 Oct 2014, 04:00

After fighting this for a long time today, I decided it must be a bug and filed a bug report. I then went about implementing winsock. First, I converted from QByteArray back to a char to send out winsock, but it had the problem also. After eliminating QByteArray all together, it works!!!!

You can see the code over on the "bug" report.

https://bugreports.qt-project.org/browse/QTBUG-42046

I would still appreciate comment from someone who knows more then me as to what I actually did wrong, or if there is a bug in QByteArray.

zcybercomputing · wrote on 21 Oct 2014, 23:46

Ok, looks like I had a case of using pointers when I shouldn't have. The problem is now resolved by forcing a deep copy. I was also under the impression that the UDP packets should have an appearance in wireshark that wasn't accurate.

SGaist · wrote on 22 Oct 2014, 23:22

Can you elaborate on your need for a deep copy ?

zcybercomputing · wrote on 23 Oct 2014, 03:45

[quote author="SGaist" date="1414020162"]Can you elaborate on your need for a deep copy ?[/quote]

stream::make_udp_packet() gets called for every transport stream packet. Since there are 7 of those in each datagram, the variable that is used to store the packets must be a copy instead of a pointer. If it is a pointer, like the code above, then the previous packet gets overwritten and every datagram will be the same collection of 7 copies of the same .ts packet.

QByteArray can operate as a deep copy, or by pointer reference.  When I first implemented it, I unknowingly used the pointer reference.  Because I made a wrong assumption of how the packets should look in wireshark, I went on a tangent of thinking there was a problem with QUdpSocket.

My code now works, I just discovered that I need to send datagrams between 2-3 ms apart.  This means I have to re implement my program to use something other then QTimer.  That will have to wait until I figure out how to patch ffmpeg to enable decode on TV sets.  (the eventual destination for these packets)  After that, ffmpeg will probably handle all of this, unless it has to much overhead.

SGaist · wrote on 23 Oct 2014, 07:26

Unless I'm mistaken you could also use gstreamer or libVLC which might provide what you need

zcybercomputing · wrote on 23 Oct 2014, 13:02

We are currently using libvlc, but both libvlc and ffmpeg do not implement pcr correctly for the TV. VLC is not willing to take the patch, and ffmpeg is used as the streaming engine for another program I would like to use. Currently each packet has to be modified in our program to make it work. This is why I am now researching how best to incorporate the correct pcr in ffmpeg.

SGaist · wrote on 23 Oct 2014, 22:58

I see, good luck !