> On 2 Mar 2018, at 14:34, Lukáš Hrázký <lhrazky@xxxxxxxxxx> wrote:
>
> On Thu, 2018-03-01 at 16:47 +0100, Christophe de Dinechin wrote:
>>> On 1 Mar 2018, at 16:14, Lukáš Hrázký <lhrazky@xxxxxxxxxx> wrote:
>>>
>>> On Wed, 2018-02-28 at 16:43 +0100, Christophe de Dinechin wrote:
>>>> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>>>
>>>> Isolating classes in separate files makes parallel builds faster,
>>>> facilitates code reuse and minimizes the chances of patch conflicts.
>>>>
>>>> Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>>> ---
>>>> src/Makefile.am | 2 ++
>>>> src/frame-log.cpp | 45 +++++++++++++++++++++++++++++++++++++
>>>> src/frame-log.hpp | 43 +++++++++++++++++++++++++++++++++++
>>>> src/spice-streaming-agent.cpp | 52 +------------------------------------------
>>>> 4 files changed, 91 insertions(+), 51 deletions(-)
>>>> create mode 100644 src/frame-log.cpp
>>>> create mode 100644 src/frame-log.hpp
>>>>
>>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>>> index 4a03e5e..5e36e1f 100644
>>>> --- a/src/Makefile.am
>>>> +++ b/src/Makefile.am
>>>> @@ -57,6 +57,8 @@ spice_streaming_agent_SOURCES = \
>>>> jpeg.hpp \
>>>> stream.cpp \
>>>> stream.hpp \
>>>> + frame-log.hpp \
>>>> + frame-log.cpp \
>>>> errors.cpp \
>>>> x11-cursor.hpp \
>>>> x11-cursor.cpp \
>>>> diff --git a/src/frame-log.cpp b/src/frame-log.cpp
>>>> new file mode 100644
>>>> index 0000000..53751be
>>>> --- /dev/null
>>>> +++ b/src/frame-log.cpp
>>>> @@ -0,0 +1,45 @@
>>>> +/* Class to log frames as they are being streamed
>>>> + *
>>>> + * \copyright
>>>> + * Copyright 2018 Red Hat Inc. All rights reserved.
>>>> + */
>>>> +
>>>> +#include "frame-log.hpp"
>>>> +#include "hexdump.h"
>>>> +
>>>> +#include <spice-streaming-agent/errors.hpp>
>>>> +
>>>> +#include <syslog.h>
>>>> +#include <inttypes.h>
>>>> +#include <errno.h>
>>>> +
>>>> +namespace spice
>>>> +{
>>>> +namespace streaming_agent
>>>> +{
>>>> +
>>>> +FrameLog::FrameLog(const char *filename, bool binary)
>>>> + : log(filename ? fopen(filename, "wb") : NULL), binary(binary)
>>>> +{
>>>> + if (filename && !log) {
>>>> + throw OpenError("failed to open hexdump log file", filename, errno);
>>>> + }
>>>> +}
>>>> +
>>>> +FrameLog::~FrameLog()
>>>> +{
>>>> + if (log)
>>>> + fclose(log);
>>>> +}
>>>> +
>>>> +void FrameLog::dump(const void *buffer, size_t length)
>>>> +{
>>>> + if (binary) {
>>>> + fwrite(buffer, length, 1, log);
>>>> + } else {
>>>> + fprintf(log, "%" PRIu64 ": Frame of %zu bytes:\n", get_time(), length);
>>>> + hexdump(buffer, length, log);
>>>> + }
>>>> +}
>>>> +
>>>> +}} // namespace spice::streaming_agent
>>>> diff --git a/src/frame-log.hpp b/src/frame-log.hpp
>>>> new file mode 100644
>>>> index 0000000..09dbd4a
>>>> --- /dev/null
>>>> +++ b/src/frame-log.hpp
>>>> @@ -0,0 +1,43 @@
>>>> +/* Class to log frames as they are being streamed
>>>> + *
>>>> + * \copyright
>>>> + * Copyright 2018 Red Hat Inc. All rights reserved.
>>>> + */
>>>> +#ifndef SPICE_STREAMING_AGENT_FRAME_LOG_HPP
>>>> +#define SPICE_STREAMING_AGENT_FRAME_LOG_HPP
>>>> +
>>>> +#include <stdio.h>
>>>> +#include <stdint.h>
>>>> +#include <sys/time.h>
>>>> +
>>>> +namespace spice {
>>>> +namespace streaming_agent {
>>>> +
>>>> +/* returns current time in micro-seconds */
>>>> +static uint64_t get_time(void)
>>>> +{
>>>> + struct timeval now;
>>>> +
>>>> + gettimeofday(&now, NULL);
>>>> +
>>>> + return (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_usec;
>>>> +
>>>> +}
>>>
>>> Why in the header?
>>
>> After splitting, it’s used in two locations that are in different .cpp files.
>
> Well, the definition should be moved to a .cpp file and it seems to me
> then this function belongs into somethings like utils.hpp.
>
> Also (more likely a followup, unless it would spare us creating a
> utils.hpp?), std::crono could be used? I need to study it a bit, not
> too familiar with it, so I am not sure (Frediano mentioned to me some
> problem with that which I forgot).
Good idea.
I’m interested in what the problem is for the use cases we have (which is basically logging a time)
>
> (I am also fine leaving it as-is and fixing it in a followup
> alltogether, as I can see some things snowballing for you and you need
> to stop somewhere.)
Indeed.
>
>>>
>>>> +
>>>> +class FrameLog
>>>> +{
>>>> +public:
>>>> + FrameLog(const char *filename, bool binary = false);
>>>> + ~FrameLog();
>>>> +
>>>> + operator bool() { return log != NULL; }
>>>> + void dump(const void *buffer, size_t length);
>>>> +
>>>> +private:
>>>> + FILE *log;
>>>> + bool binary;
>>>> +};
>>>> +
>>>> +}} // namespace spice::streaming_agent
>>>> +
>>>> +#endif // SPICE_STREAMING_AGENT_ERRORS_HPP
>>>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>>>> index 2264af2..424db95 100644
>>>> --- a/src/spice-streaming-agent.cpp
>>>> +++ b/src/spice-streaming-agent.cpp
>>>> @@ -7,8 +7,8 @@
>>>> #include "concrete-agent.hpp"
>>>> #include "stream.hpp"
>>>> #include "message.hpp"
>>>> +#include "frame-log.hpp"
>>>> #include "x11-cursor.hpp"
>>>> -#include "hexdump.h"
>>>> #include "mjpeg-fallback.hpp"
>>>>
>>>> #include <spice/stream-device.h>
>>>> @@ -45,17 +45,6 @@ namespace spice
>>>> namespace streaming_agent
>>>> {
>>>>
>>>> -/* returns current time in micro-seconds */
>>>> -static uint64_t get_time(void)
>>>> -{
>>>> - struct timeval now;
>>>> -
>>>> - gettimeofday(&now, NULL);
>>>> -
>>>> - return (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_usec;
>>>> -
>>>> -}
>>>> -
>>>> class FormatMessage : public Message<StreamMsgFormat, FormatMessage, STREAM_TYPE_FORMAT>
>>>> {
>>>> public:
>>>> @@ -85,45 +74,6 @@ public:
>>>> }
>>>> };
>>>>
>>>> -class FrameLog
>>>> -{
>>>> -public:
>>>> - FrameLog(const char *filename, bool binary = false);
>>>> - ~FrameLog();
>>>> -
>>>> - operator bool() { return log != NULL; }
>>>> - void dump(const void *buffer, size_t length);
>>>> -
>>>> -private:
>>>> - FILE *log;
>>>> - bool binary;
>>>> -};
>>>> -
>>>> -
>>>> -FrameLog::FrameLog(const char *filename, bool binary)
>>>> - : log(filename ? fopen(filename, "wb") : NULL), binary(binary)
>>>> -{
>>>> - if (filename && !log) {
>>>> - throw OpenError("failed to open hexdump log file", filename, errno);
>>>> - }
>>>> -}
>>>> -
>>>> -FrameLog::~FrameLog()
>>>> -{
>>>> - if (log)
>>>> - fclose(log);
>>>> -}
>>>> -
>>>> -void FrameLog::dump(const void *buffer, size_t length)
>>>> -{
>>>> - if (binary) {
>>>> - fwrite(buffer, length, 1, log);
>>>> - } else {
>>>> - fprintf(log, "%" PRIu64 ": Frame of %zu bytes:\n", get_time(), length);
>>>> - hexdump(buffer, length, log);
>>>> - }
>>>> -}
>>>> -
>>>> }} // namespace spice::streaming_agent
>>>>
>>>> bool quit_requested = false;
>>>
>>> _______________________________________________
>>> Spice-devel mailing list
>>> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
>>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel